Skip to content

Fix rate text descriptions to use tariff range#3484

Merged
springfall2008 merged 2 commits intospringfall2008:mainfrom
warksit:fix-rate-coloring
Mar 5, 2026
Merged

Fix rate text descriptions to use tariff range#3484
springfall2008 merged 2 commits intospringfall2008:mainfrom
warksit:fix-rate-coloring

Conversation

@warksit
Copy link
Contributor

@warksit warksit commented Mar 4, 2026

Summary

  • Rate text descriptions ("cheap", "expensive", etc.) in the status message now compare rates to other rates on the tariff, instead of using the battery charging threshold
  • Previously, the charging threshold made most rates appear "expensive" or "very expensive" on multi-tier tariffs like Cosy Octopus, even for the cheapest available slots
  • Now the cheapest third of rates are "cheap", middle third "expensive", and top third "very expensive"

Test plan

  • Cosy Octopus: 14p = cheap, 28p = expensive, 43p = very expensive
  • Agile: rates spread across the three bands proportionally
  • Flat rate tariff: shows "fixed"
  • Negative/free rates still handled correctly

🤖 Generated with Claude Code

Remove charge threshold override in band_rate_text so rate descriptions
reflect the actual tariff structure. On multi-tier tariffs like Cosy
Octopus, 14p slots now show as "cheap" instead of "expensive".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
text = "expensive"
else:
text = "very expensive"
rate_frac = (rate - self.rate_min) / (self.rate_max - self.rate_min)
Copy link
Owner

Choose a reason for hiding this comment

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

This could give a divide by zero error on a fixed rate tariff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will add a check for that and resolve the test error (thought I had run them before submitting, sorry)

The export branch of band_rate_text referenced a removed local variable
(export_cost_threshold), causing a NameError. Switched export to use the
same rate_frac approach as import with 4 bands (very low/low/good/very good).

Added tests covering flat rate, Cosy Octopus, Flux import/export, and
edge cases (free, negative, zero).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@warksit
Copy link
Contributor Author

warksit commented Mar 4, 2026

Thanks for the review. The divide-by-zero was already guarded (the rate_min == rate_max check returns "fixed" before reaching the division), but there was a real bug: I removed the export_cost_threshold local variable definition when switching import to rate_frac, but left the export branch still referencing it — causing a NameError on any export rate lookup. Fixed in f6d4b6c.

Export now uses the same rate_frac approach with 4 bands (very low / low / good / very good, at 0.25/0.5/0.75 boundaries). Also added dedicated band_rate_text tests covering flat rate, Cosy Octopus, and Flux tariffs (import + export).

@springfall2008 springfall2008 requested a review from Copilot March 5, 2026 18:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@springfall2008 springfall2008 requested a review from Copilot March 5, 2026 18:59
@springfall2008 springfall2008 merged commit 4a1dd71 into springfall2008:main Mar 5, 2026
1 check 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.

3 participants