feat(tariff): add Tariff V2 rate resolver - #103
Conversation
Add get_tariff_periods, a pure offline resolver over the raw tariff_content_v2 object, returning the current buy/sell rate plus the next boundary (including buy/sell season start/end boundaries, not just weekly period-grid starts). Ports the Home Assistant teslemetry integration's season/day-of-week/midnight-cross matching logic without its file - the reference crashes on real data's toHour: 24 end-of-day periods because it constructs datetime.replace(hour=24); this resolver represents instants as minute-of-week instead, which sidesteps that crash entirely, and treats a 0.0 sell price as a real value rather than a missing one. Includes a live-shaped 48-half-hour-period fixture plus tests for season year-crossing and boundary delimiting, day-of-week wrap, midnight-crossing periods, the toHour: 24 regression, absent sell_tariff, missing rates, and naive-now rejection. Documents the helper's usage in docs/fleet_api_energy_sites.md.
Tighten unwrap_tariff_v2's bare-object fallback to require the minimal Tariff V2 shape (seasons + energy_charges) before accepting it, raising InvalidResponse otherwise - an empty or unrelated dict is malformed data, not "no tariff configured". Bound current_start/next_change to the matched buy/sell window's own start/end instead of the next period-start anywhere in the grid, so a sparse tariff (gaps between periods) transitions to no active period at the correct time instead of carrying the old rate across the gap. Added tests confirming season re-resolution across multiple season transitions within one horizon walk, DST-safe wall-clock boundary construction across a spring-forward transition, and the existing season-key-present-but-no-rates ALL fallback - all three already resolved correctly and now have regression coverage.
Cosmetic line-wrap cleanup only, no behavior change.
…back correctness Round-3 Codex re-review (node sibling) flagged three more scenarios: sell gaps affecting next_change, wall-clock day derivation across a DST fall-back, and current_start honoring the later of buy/sell starts. All three already resolve correctly under the existing minute-of-week + wall-clock-timedelta design (verified interactively before writing these tests) - this commit adds the regression coverage rather than a behavior change.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02f3d67aa9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return cast( | ||
| "dict[str, Any]", cast("dict[str, Any]", inner)["tariff_content_v2"] | ||
| ) |
There was a problem hiding this comment.
When a response or tou_settings envelope contains tariff_content_v2 as {} or any other dict without the required seasons and energy_charges, this return accepts it before the minimal-shape check below can run (the top-level tariff_content_v2 branch has the same issue). Callers passing a malformed/empty API body then get None or missing prices from get_tariff_periods() instead of the documented InvalidResponse, so validate the extracted dict before returning it.
Useful? React with 👍 / 👎.
… path
The seasons+energy_charges check only ran on the bare-dict fallback -
the tariff_content_v2 top-level key, response envelope, and
tou_settings envelope paths all returned their extracted dict without
it, so an enveloped empty {} silently passed through instead of
raising InvalidResponse. Refactored to extract a single candidate
across all four paths and validate it once, so the check is now
identical everywhere.
|
Fixed the Codex P2 finding ( Refactored |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7166727ceb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if next_resolved is None: | ||
| break |
There was a problem hiding this comment.
When the active buy period is followed by a gap, this break stops the horizon walk entirely, so upcoming omits later periods that still fall inside the requested horizon. For example, with the sparse daily 06:00-09:00 tariff already covered by SparsePeriodTests, calling at 07:00 with horizon_hours=48 returns only today's morning period and misses tomorrow morning's matching period; callers using the resolver to plan future rates get an incomplete schedule. Advance through gaps to the next tariff boundary instead of treating the first unresolved instant as the end of the horizon.
Useful? React with 👍 / 👎.
When the moment right after the currently-active period fell in a gap (no buy period covers it - e.g. a sparse 06:00-09:00 daily tariff, or a season that only covers part of the year), the horizon walk hit _resolve_at returning None and broke immediately, so upcoming silently omitted every later in-horizon period past the first gap. Added _next_resolvable, which finds the next moment where the buy side resolves again - either the grid's own next window start (reusing the same nearest-boundary search already used for an inactive sell grid) when a season still covers the date, or the next season's start (reusing the existing adjacent-season-boundary search) when no season covers it at all. The walk now records the gap itself as an explicit unresolved TariffPeriod (both sides None) and continues from the gap's end, so upcoming stays a contiguous timeline with no silent hole, and clips cleanly to the horizon deadline if the tariff never resumes within it. Added regression tests: a sparse daily period whose gap spans into the next day, a weekend-only period whose gap spans a full working week, and a season that only covers part of the year (clipped-at-deadline and reaches-next-season-start cases).
A single jump out of a gap (e.g. to a new season's start) can itself land in another gap - that season's own grid might not cover midnight either. The previous fix only handled one jump per gap and then broke if the landing point was still unresolved. Now the walk keeps calling _next_resolvable until the buy side actually resolves or the horizon runs out, merging every chained jump into one contiguous unresolved segment. Added a regression test where a season only has a 06:00-09:00 daily period: jumping to next season's start (00:00) still isn't resolvable, requiring a second jump to that season's own first window.
|
Fixed the Codex P2 finding ( Added A follow-up review round caught one more edge case in that fix: the intra-season gap boundary needs to be the earlier of the grid's next window start or the season's own end, so a different season starting sooner is picked up correctly rather than skipped past. Audited the whole upcoming-walk exhaustively per the request - added regression tests for a sparse daily gap, a weekend-only period's 5-day gap, a partial-year season (both clip-at-deadline and reach-next-season-start), and the chained-gap case. Full suite (508 tests), pyright strict, and ruff are all clean; CI is green. |
1 participant
Intent
Revalidate PR #103 (python-tesla-fleet-api) after fixing a second Codex P2 finding on that PR: get_tariff_periods' horizon-walk (line ~146) broke entirely the first time it hit a gap - a moment no buy period covers, e.g. a sparse 06:00-09:00 daily tariff, or a season that only covers part of the year - so 'upcoming' silently omitted every later in-horizon period past the first gap (a 48-hour horizon called at 07:00 on a 06:00-09:00-only tariff returned just today, missing tomorrow morning entirely).
Fix: added _next_resolvable, which finds the next moment the buy side resolves again - either the grid's own next window start (reusing the same nearest-boundary search already used for an inactive sell grid) when a season still covers the date, or the next season's start (reusing the existing adjacent-season-boundary search) when no season covers the date at all. The walk now loops calling _next_resolvable until the buy side truly resolves or the horizon runs out - a single jump (e.g. landing exactly on a new season's start) can itself land in ANOTHER gap if that season's own grid doesn't cover midnight either, so this had to chain through multiple jumps, not just one. Every gap (however many jumps it takes to cross) is recorded as one merged, contiguous unresolved TariffPeriod (both buy and sell None) so upcoming stays a complete timeline with no silent holes, and clips cleanly to the horizon deadline if the tariff never resumes within it.
Audited the whole upcoming-walk exhaustively per the request: added regression tests for a sparse daily period whose gap spans into the next day, a weekend-only period whose 5-day (Mon-Fri) gap requires skipping straight to the following weekend, a season that only covers part of the year (both the clipped-at-deadline case and the reaches-next-season-start case within a longer horizon), and the chained-gap case where a season boundary jump itself lands in another gap requiring a second jump. Full suite (507 tests), pyright strict, and ruff all clean locally before this run. PR #103 already exists and is open for this branch (#103) - this run should update/revalidate that existing PR, not create a new one.
What Changed
Risk Assessment
✅ Low: The correction now advances to the earlier of the next weekly window and the current season’s end, preserving the complete contiguous timeline across season transitions without introducing a material new risk.
Testing
The focused tariff suite and full runtime suite passed, and direct API verification demonstrated contiguous, deadline-covering timelines across sparse daily and chained seasonal gaps. JSON is the product-level evidence because this change exposes a Python library API rather than a rendered UI.
Evidence: Tariff gap timeline evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
tesla_fleet_api/tariff.py:420- When a gap occurs inside a season, this returns the next weekly grid boundary without considering that the season may end sooner. If season A is currently in a gap until Friday but season B begins Thursday with a resolvable rate, the walker jumps directly to Friday and incorrectly records Thursday as unresolved. Return the earliest of the next grid window and the current season end, allowing the loop to re-resolve at the season transition.🔧 Fix: Respect season boundaries when advancing tariff gaps
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
uv run pytest tests/test_tariff.py -quv run pytest -qManualuv run pythonAPI exercise ofget_tariff_periodsfor a 48-hour sparse daily tariff and a 310-day chained season/intraday gap, recording JSON evidence and checking timeline contiguity and deadline coverage✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.