fix(ranking): dedupe by economic fingerprint before top-K cut#149
Merged
Conversation
Live UAT 2026-05-24: after beta.4's per-tick explanation rebuild and the NEMWeb regex fixes, the alternatives sensor produced top-K results but every entry was a marketing-channel variant of one underlying plan. Observed: Origin Go Variable - New and Moving Customers Only (Residential Demand) Origin Affinity Variable - Comparable (Residential Demand w Dedicated Circuit) Origin Affinity Variable - Comparable (Residential Demand) Origin Affinity Variable - One Click Switch (Residential Demand) Origin Affinity Variable - Electricity Wizard (Residential Demand) All five carry identical peak rate + daily supply charge. The score function ``peak * 0.7 + supply * 0.3`` was working correctly; the underlying offer was genuinely the cheapest — but ranking it five times makes the user think they have a 5-way switch decision when they really have one. Fix: ``_economic_fingerprint`` = ``(peak_cents, supply_cents)``. ``cheap_rank`` dedupes by fingerprint BEFORE the top-K cut. First-seen at each fingerprint wins → deterministic given the input order from ``fetch_plans_for_retailer``. Subtle rate-shape differences (TOU windows, step thresholds, demand charges) are intentionally NOT in the fingerprint. Those are the right job for ``deep_rank`` against the user's actual consumption — same- fingerprint plans diverge there when consumption hits different windows. 3 new regression tests in ``TestCheapRank``: - test_identical_economic_fingerprint_collapses_to_one_representative - test_first_seen_wins_at_each_fingerprint - test_unscorable_plan_does_not_pollute_fingerprint_set Manifest bumped to 1.6.0-beta.5. Silver-checklist version assertion updated to match. Full test suite: 1133 passing.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a deduplication mechanism in the cheap_rank function to ensure that multiple marketing variants of the same electricity plan do not saturate the top-K results. Plans are now filtered by an "economic fingerprint" consisting of peak and supply rates, with the first-seen plan retained. The changes include comprehensive regression tests and a version bump to 1.6.0-beta.5. Review feedback suggests optimizing the ranking loop to eliminate redundant data extraction by deriving the rank score directly from the fingerprint components.
Comment on lines
291
to
299
| score = cheap_rank_score(p) | ||
| if score is None: | ||
| continue | ||
| fingerprint = _economic_fingerprint(p) | ||
| if fingerprint is not None and fingerprint in seen_fingerprints: | ||
| continue | ||
| if fingerprint is not None: | ||
| seen_fingerprints.add(fingerprint) | ||
| scored.append((score, p)) |
There was a problem hiding this comment.
The current implementation of cheap_rank performs redundant work by calling both cheap_rank_score(p) and _economic_fingerprint(p). Both functions internally call _extract_peak_rate_cents and _extract_daily_supply_cents, which involve multiple loops over the plan's tariff structure and Decimal conversions.
Since _economic_fingerprint returns the raw components, you can use it to both deduplicate and calculate the score in a single pass. This avoids redundant extraction calls and simplifies the logic by removing the unnecessary if fingerprint is not None checks (as the preceding score is None check already implies the extraction succeeded).
Suggested change
| score = cheap_rank_score(p) | |
| if score is None: | |
| continue | |
| fingerprint = _economic_fingerprint(p) | |
| if fingerprint is not None and fingerprint in seen_fingerprints: | |
| continue | |
| if fingerprint is not None: | |
| seen_fingerprints.add(fingerprint) | |
| scored.append((score, p)) | |
| fingerprint = _economic_fingerprint(p) | |
| if fingerprint is None or fingerprint in seen_fingerprints: | |
| continue | |
| seen_fingerprints.add(fingerprint) | |
| peak, supply = fingerprint | |
| score = peak * _PEAK_WEIGHT + supply * _SUPPLY_WEIGHT | |
| scored.append((score, p)) |
Artic0din
added a commit
that referenced
this pull request
May 24, 2026
…#153) * fix(stack): four gemini-review findings I missed across beta.4-beta.8 Ryan caught that I'd been merging today's PRs with --auto without reading gemini-code-assist reviews. These are the legitimate findings from those reviews, fixed in one batch. #148 (gemini): per-tick build_explanation duplicates the rollover branch. Beta.4 added a per-tick rebuild but left the daily-rollover rebuild in place. At midnight the rollover branch built the "yesterday's summary" snapshot, then the per-tick rebuild a few seconds later clobbered it with the post-reset all-zeros snapshot. Per-tick rebuild fires on the midnight tick too, so the rollover one was redundant AND actively harmful. Removed. #147 (gemini): _pick_latest_dispatch_file lex-sort is case-sensitive. _FILE_RE uses re.IGNORECASE — could capture mixed-case filenames. sorted(matches)[-1] then compared Unicode codepoints; uppercase sorts before lowercase. A hypothetical mixed-case listing would put an older uppercase file last after sort. NEMWeb today is all uppercase but the contract should match the regex's tolerance. key=str.upper normalises. #150 (gemini): rebuild_engine never re-reads _grid_power_entity. The options→data fallback added in beta.6 fires once at __init__ and never refreshes. Users editing the grid sensor via options-flow saw the integration silently keep pointing at the prior entity until HA restart. rebuild_engine is the options-update path — now re-resolves. #152 (gemini): reset_today service silently succeeds with no entries. Silver action-exceptions rule: service handlers should raise when they cannot perform the action. A user with all entries in failed-load state would see "Successfully executed pricehawk.reset_today" and observe no dashboard change. Now raises HomeAssistantError telling them to reload the integration. PR #146 (export-credit bullet) and PR #149 (loop optimization) are real findings too but lower priority — left as future work. Manifest bumped to 1.6.0-beta.9. Silver-checklist version assertion updated to match. Full test suite: 1140 passing. * fix(coordinator): drop duplicate _grid_power_entity overwrite in rebuild_engine Per gemini review of PR #153: the fallback I added at the TOP of rebuild_engine was being silently negated by a duplicate options-only assignment at the END of the same function. Removed the second assignment; kept a comment marker so this isn't accidentally reintroduced. Bug effect: rebuild_engine fired on every options-flow save would overwrite my fix with the empty-string default if entry.options didn't have CONF_GRID_POWER_SENSOR. Same regression as the original bug beta.6 was meant to fix. NOTE: gemini also flagged that rebuild_engine creates new provider instances on every options update, losing today's daily accumulators (import_kwh_today, import_cost_today_c, etc.). That's a real concern but a bigger refactor — left for a follow-up PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third post-beta UAT hotpatch. Top-K alternatives were dominated by marketing-channel variants of the same underlying plan (5x "Origin Affinity Variable - …" with identical economics).
cheap_ranknow collapses plans sharing(peak_cents, supply_cents)to a single representative before the top-K cut. First-seen wins.3 new tests. 1133 passing locally.