Add getTariffPeriods: pure Tariff V2 buy/sell rate resolver - #7
Conversation
Mirrors the python-tesla-fleet-api sibling's shape and edge cases (season year-cross, day-of-week wrap, midnight cross, toHour:24 normalization, ALL fallback, key-presence price lookup) using a JS-appropriate approach: an IANA timeZone string plus Intl.DateTimeFormat instead of a tz-aware Date. Also fixes TariffContentV2.seasons, which was mistyped as an array when the real payload is an object keyed by season name.
![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: 8dbab2bd60
ℹ️ 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 (instances.length === 0) return null; | ||
| const current = findContaining(instances, nowAbs); | ||
| if (!current) return null; | ||
| const nextChangeOffset = findNextStart(instances, nowAbs) ?? current.end; |
There was a problem hiding this comment.
For tariffs that do not cover every minute, this picks the next future period start even when the current interval ends earlier. For example, with a 10:00-11:00 period and the next period at 17:00, a lookup at 10:30 reports nextChange as 17:00, so callers keep using the 10:00 rate through a gap where findContaining would return no rate. Consider taking the earlier of current.end and the next start so gaps transition to the empty/null rate at the correct time.
Useful? React with 👍 / 👎.
| periods.push({ | ||
| start: toDate(segStart), | ||
| end: toDate(segEnd), | ||
| buy: buyInstance ? { price: lookupPrice(buyGrid.charges, buyGrid.seasonName, buyInstance.periodName), periodName: buyInstance.periodName, seasonName: buyGrid.seasonName } : EMPTY_RATE, |
There was a problem hiding this comment.
When horizonHours crosses a season boundary, the upcoming schedule still uses the buyGrid that was resolved for now, and each future segment is priced with buyGrid.seasonName. A tariff with Summer ending July 31 and Winter starting August 1 will therefore label and price August 1 segments as Summer if the caller asks for a horizon from July 31, which makes the returned forecast wrong for seasonal tariffs. The builder needs to include season boundaries and resolve the applicable season for each segment.
Useful? React with 👍 / 👎.
The fixture's real values (rates, period geometry) came from a live capture but are not sensitive; only the source site's name and id were.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52170d0595
ℹ️ 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 (!buyGrid) return null; | ||
| const sellGrid = tariff.sell_tariff ? resolveGrid(tariff.sell_tariff.seasons ?? tariff.seasons, tariff.sell_tariff.energy_charges, wallClock, nowAbs) : null; | ||
|
|
||
| const toDate = (offset: number): Date => new Date(now.getTime() + (offset - nowAbs) * 60_000); |
There was a problem hiding this comment.
For sites in DST-observing zones when a resolved boundary crosses the transition, this treats a wall-clock minute delta as elapsed UTC minutes. For example, in America/New_York on March 8, 2026 at 01:30, a 01:00-03:00 local period should end at 03:00 EDT, but adding 90 minutes to the UTC instant returns 04:00 local, so nextChange/upcoming switch rates an hour late; construct boundary dates in opts.timeZone instead.
Useful? React with 👍 / 👎.
| const seasonRates = (Object.prototype.hasOwnProperty.call(charges, seasonName) ? charges[seasonName] : charges["ALL"])?.rates; | ||
| if (!seasonRates) return null; |
There was a problem hiding this comment.
When the matching season key exists but its value has no rates (the added live fixture includes empty seasonal stubs like Winter: {} alongside ALL defaults), this branch chooses that empty object and the next line returns null without ever consulting charges.ALL. Tariffs that rely on generic ALL pricing for an otherwise matched season will report price: null for the whole season; fall back to ALL whenever the season entry lacks rates, not only when the key is absent.
Useful? React with 👍 / 👎.
… ALL fallback Address four correctness issues from the Codex PR review: - nextChange now stops at the active period's own end for a sparse tariff, instead of reporting the next period found arbitrarily far in the future. - The upcoming schedule re-resolves the season on each calendar day instead of reusing the season resolved for `now`, so a horizon crossing a season boundary re-prices and re-labels correctly. - Boundary instants are now reconstructed from their wall-clock label via an iterative UTC-offset solve, instead of adding elapsed real minutes to `now` - the two diverge across a DST transition. - Price lookup falls back to the ALL season/period bucket when the matched season key is present but carries no rates at all (not just when the key is absent). This replaces the point-in-time resolver's minute-of-week/instances model with a day-by-day walk (dayPeriods), which both fixes the season re-resolution issue and unifies the logic used for the current-rate lookup and the upcoming-schedule builder.
|
Addressed all four - thanks, these were real:
While rewriting the point-in-time resolver to fix #2, I unified it with the All of these apply to the |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e38b07bc7
ℹ️ 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 new Date(wallClockToUtcMillis(target.year, target.month, target.day, target.hour, target.minute, timeZone)); | ||
| }; | ||
|
|
||
| const nextChangeGM = sell ? Math.min(buy.nextChangeGM, sell.nextChangeGM) : buy.nextChangeGM; |
There was a problem hiding this comment.
When a sell_tariff exists but its schedule is currently in a gap, resolveAt returns null, so this falls back to buy.nextChangeGM only. For a buy all-day tariff with a sell/export window from 17:00-19:00, a lookup at 16:30 returns an empty sell rate but a nextChange at the buy boundary, often midnight, so callers can cache the empty sell rate past 17:00 instead of observing the sell price start; track the next sell boundary even when no sell period currently contains now.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| const segments: ScheduleSegment[] = []; | ||
| let dayStartGM = -wallClock.minuteOfDay; | ||
| while (dayStartGM < horizonMinutes) { | ||
| const dayDate = new Date(now.getTime() + dayStartGM * 60_000); |
There was a problem hiding this comment.
The new evidence is that collectSegments still computes future local midnights by adding wall-clock minutes as elapsed UTC milliseconds. On a fall-back day such as America/New_York on 2026-11-01 at 00:30, the second loop iteration uses dayStartGM = 1410, but now + 1410m formats as 2026-11-01 23:00 Sunday, so the segment later rendered by toDate(1410) as Monday 00:00 is populated with Sunday's season/weekday rates; derive the day label via wall-clock shifting/conversion instead.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| const resolution: TariffResolution = { | ||
| buy: buy.rate, | ||
| sell: sell ? sell.rate : EMPTY_RATE, | ||
| currentStart: toDate(buy.currentStartGM), |
There was a problem hiding this comment.
When the sell tariff has a different schedule from the buy tariff and both are active, this reports the buy period's start even if the sell period started later. For example, an all-day buy rate with a 17:00-19:00 sell peak queried at 18:00 returns the correct sell price but currentStart at 00:00, so consumers treating currentStart/nextChange as the valid interval for the returned buy+sell pair overstate the sell price's interval; use the later of buy.currentStartGM and sell.currentStartGM.
Useful? React with 👍 / 👎.
…tStart Address three more Codex review findings: - nextChange now folds in the sell grid's next boundary even when sell is currently sitting in a gap (e.g. an export window not open yet), instead of silently falling back to buy's boundary alone. - Forward-looking day derivation (walking the upcoming horizon one day at a time, and resolveAt's tomorrow peek) now goes through pure wall-clock arithmetic instead of adding elapsed real minutes to a UTC instant, which landed on the wrong calendar day across a DST fall-back (25-hour) or spring-forward (23-hour) day. - currentStart is now the later of buy's and sell's own period start, not just buy's, when the two schedules differ. Unifies the point-in-time resolver and the sell-gap lookup around one `scheduleAt` function, since both need "next boundary regardless of whether a period is active right now".
|
Third round addressed - all three were real:
While fixing #1 I merged the old All three apply to the
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7973e5a6bd
ℹ️ 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".
|
|
||
| // When buy and sell schedules differ, the interval the returned pair is valid for only began | ||
| // once BOTH sides' current periods had started - the later of the two starts, not just buy's. | ||
| const currentStartGM = sellSchedule?.current ? Math.max(buySchedule.current.trueStartMin - wallClock.minuteOfDay, sellSchedule.current.trueStartMin - wallClock.minuteOfDay) : buySchedule.current.trueStartMin - wallClock.minuteOfDay; |
There was a problem hiding this comment.
When a sell tariff is currently in a gap after a sell period ended (for example, buy is all-day, sell/export is 17:00-19:00, and the caller queries at 20:00), this branch falls back to the buy period start because sellSchedule.current is null. The returned sell rate is empty, but that buy+sell pair only became valid at 19:00, so consumers using currentStart/nextChange as the interval will backdate the empty sell rate to midnight; track the sell gap's previous boundary and take the later start even when no sell period is active.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
…action Address round-4 finding: when sell is currently in a gap after a period ended (e.g. an export window that already closed), currentStart now reports the gap's own start (the previous period's end), not buy's period start - the returned empty sell rate was previously backdated to whenever buy's period began. scheduleAt now computes sinceGM symmetrically with nextChangeGM: both are defined whether or not a period is currently active (bounded to one day of lookback/lookahead), and both are nullable so a grid with no data in that window at all (never resolves a season, or resolves one with zero periods) is excluded from the combination instead of forcing a bogus default. Exhaustively covers the buy/sell interaction with tests: sell started after buy, buy started after sell, sell gap before its first period, sell gap after its last period, buy's own boundary winning the nextChange min, a sell season that doesn't cover today at all, a sell season with zero periods ever, and sell_tariff omitting its own seasons (falling back to buy's geometry).
|
Fixed, and per the request I did a full exhaustive audit of the buy/sell current/next interaction rather than a point patch, to break the review loop. The round-4 finding: confirmed real. When sell is in a gap after a period ended (buy all-day, sell/export 17:00-19:00, queried at 20:00), The fix: Exhaustive audit of the buy/sell interaction - added tests for every combination I could enumerate:
32 tariff tests now, all green, plus the existing 56 from the rest of the suite (88 total). Build gate ( Mirrors to
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a596ec175
ℹ️ 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".
| daily_charges?: Record<string, any>[]; | ||
| demand_charges?: Record<string, any>; | ||
| energy_charges: EnergyCharges; | ||
| seasons: Record<string, Season>; |
There was a problem hiding this comment.
When a tariff's sell_tariff relies on the buy tariff's season geometry, the resolver explicitly supports that by falling back to tariff.seasons, and the new tests exercise a sell_tariff object with only energy_charges. Keeping seasons required in the exported SellTariff type makes that valid payload fail TypeScript checking for consumers unless they cast through any, so the type should mark seasons optional to match the runtime/API shape.
Useful? React with 👍 / 👎.
The resolver already falls back to the buy tariff's own seasons/geometry when sell_tariff omits seasons (tariff.sell_tariff?.seasons ?? tariff.seasons), and a test exercises exactly that shape - but the exported type still required seasons, so a valid payload failed typechecking for consumers.
|
Fixed - Worth noting for anyone reading this thread later: this repo's Mirrors to That should be everything - thanks for the thorough pass across all five rounds. |
1 participant
What
src/tariff.ts:getTariffPeriods(tariff, now, opts?), a pure, offline resolver for the Tariff V2 time-of-use rate schedule (tariff_content_v2fromsite_info/get_energy_tariff). Returns the current buy/sell rate,currentStart,nextChange, and currency in one call - mirrors the shape and edge cases of thepython-tesla-fleet-apisibling of the same name.tariffcarries no timezone, so the caller passesopts.timeZone(an IANA string, e.g.site_info.installation_time_zone); wall-clock parts come fromIntl.DateTimeFormat(...).formatToParts(now)since JS has no tz-awareDate.toHour: 24/toMinute: 60normalization, buy/sellseason -> "ALL"andperiod -> "ALL"fallback, and key-presence price lookup so a real0.0rate isn't dropped as falsy.opts.horizonHoursalso returns anupcominglist of forward periods.TariffContentV2(src/types/site_info.ts):seasonswas typed as an array but the real payload is an object keyed by season name; addedSeason/TouPeriods/EnergyCharges/SellTariffinterfaces so buy and sell lookups are typed instead ofany.getTariffPeriodsand theTariffRate/TariffPeriod/TariffResolutiontypes fromsrc/index.ts.test/tariff.test.ts): a live Tariff V2 capture (site name/id anonymized) (test/fixtures/sample-tariff.ts) plus synthetic fixtures covering each edge case above, including thetoHour: 24regression and missing-timezone/missing-rate/no-season-match/absent-sell-tariff cases.