Skip to content

feat(planner): persist forecast trust and battery export permission - #968

Open
frahlg wants to merge 2 commits into
masterfrom
feat/planner-forecast-trust
Open

feat(planner): persist forecast trust and battery export permission#968
frahlg wants to merge 2 commits into
masterfrom
feat/planner-forecast-trust

Conversation

@frahlg

@frahlg frahlg commented Aug 21, 2026

Copy link
Copy Markdown
Member

Status

This PR is PR 1 of the household-planner beta. Core prefs are in: SQLite + /api/planner/prefs + status fields. No UI yet.

A cloud agent should continue with PR 2–4 on this branch (or stacked on it). Do not put planning docs in the repo.

Planning docs stay out of git. The full plan is in this description.

What this commit does

  • forecast_trust: cautious | balanced | bold → only pv_forecast_safety_k (2 / 1 / 0)
  • battery_export: unknown | not_allowed | allowed → existing planner_passive_arbitrage / planner_arbitrage
  • Balanced = today's default (k=1)
  • Unknown export never sells from the battery
  • Upgrade from Active: unknown, forced to passive until confirmed
  • YAML pv_forecast_safety_k wins (yaml_custom)
  • GET/POST /api/planner/prefs; same fields on /api/status
  • HA and app mode setters confirm export the same way as /api/mode

Remaining work (do this next)

PR 2 — Plan card: slider + export

  • Remove Passive/Active as primary strategy buttons in web/app.js
  • web/plan.js: slider “Follow the forecast” (Hold reserve → Trust forecast), export permission, four sentences, upgrade banner
  • Live hedge line under the slider (k·σ in watts)
  • Browser check desktop + narrow

PR 3 — Settings: simple on top, engine below

  • web/settings/tabs/planner.js: top = enabled, house reserve, soc_max
  • Closed <details> at the bottom: “Engine controls — leave these unless you are debugging.”
  • Saving with the disclosure closed must not wipe engine fields
  • Do not put engine=python or CLARABEL on top. HiGHS is implied.

PR 4 — Weather: no array onboarding

  • Map stays. No +Add on the normal path
  • Copy: production pattern from measured solar, not “orientation”
  • Existing arrays: read-only “N arrays set in config”
  • Editor only inside closed advanced if needed

PR 5 (optional, do not block beta)

  • Twins + one-liner on the Plan card from existing Quality() / Samples
  • Rename reset to “Solar system changed”

Product rules (do not break)

  • Do not call the slider risk
  • Slider never turns on battery export
  • Do not map slider onto CVaR, scenario trees, or Python-only knobs
  • Do not rewrite control/dispatch.go
  • Do not retier ModeCatalog this beta (app/HA keep Passive/Active keys)
  • Do not write planning docs into the repo
  • Do not auto-write observed peak AC into pv_rated_w

Mapping

battery_export
  unknown      → planner_passive_arbitrage
  not_allowed  → planner_passive_arbitrage
  allowed      → planner_arbitrage

forecast_trust  (only if YAML did not set pv_forecast_safety_k)
  cautious  → k = 2.0
  balanced  → k = 1.0
  bold      → k = 0.0

Copy

Slider: “Follow the forecast”. Left “Hold reserve”. Right “Trust forecast”.
Help: “Left keeps more in the battery if the sun might miss — closer to using the battery only for the house. Right follows the forecast fully. If the forecast is right, right earns more.”

Export: “Allow the battery to sell to the grid when the plan expects a worthwhile sale. Solar can still export when this is off. Check your electricity contract.”
Unknown: “Not checked — battery export stays off.”

Upgrade banner: “FTW used to sell from the battery on high-price hours. Allow that to continue?”

Plan sentences:

  • “Battery sale planned HH:MM–HH:MM.”
  • “Solar export only; the battery is not selling.”
  • “Battery export is allowed, but FTW found no worthwhile sale.”
  • “Battery sale blocked: permission is off or not checked.”

Test plan

  • go test ./internal/config ./internal/api ./internal/mpc ./cmd/ftw
  • make verify on commit
  • Browser: Plan card slider + export (PR 2)
  • Settings advanced disclosure closed does not wipe YAML (PR 3)

Note

Medium Risk
Changes live planner dispatch: PV safety k and whether the battery may sell to the grid, including an upgrade path that pauses Active-arbitrage export until the household confirms.

Overview
Replaces Passive/Active as the household Plan-card knobs with follow-the-forecast (cautious / balanced / bold → PV safety k 2 / 1 / 0) and a battery-export permission (unknown / not allowed / allowed). Balanced is the default. Unknown never sells from the battery. Sites that were on Active arbitrage migrate to unknown and stay on passive until they confirm.

Prefs live in SQLite (YAML is first-boot only). GET/POST /api/planner/prefs and /api/status expose them; setting prefs maps export onto planner_passive_arbitrage / planner_arbitrage and updates MPC safety k (explicit YAML pv_forecast_safety_k still wins). Mode changes from the web API, HA, and the app confirm export the same way.

The Plan card posts trust without flipping export, shows a live k·σ hedge line, an upgrade banner, and a one-line sale/export sentence. Settings put house reserve on top and bury engine knobs; weather keeps the map and hides array geometry behind advanced, with copy that the production pattern is learned from measured solar.

Reviewed by Cursor Bugbot for commit a54ca30. Bugbot is set up for automated code reviews on this repo. Configure here.

Household planner prefs live in SQLite like mode: forecast_trust
(cautious/balanced/bold) maps only to pv_forecast_safety_k, and
battery_export (unknown/not_allowed/allowed) maps onto the existing
passive vs active planner modes.

Balanced is today's k=1. Unknown export never sells from the battery.
An upgrade from Active arbitrage starts unknown and is forced to
passive until confirmed. YAML pv_forecast_safety_k still wins.

GET/POST /api/planner/prefs plus the same fields on /api/status.
HA and the app mode setters confirm export the same way as /api/mode.
if mm, ok := control.PlannerMPCMode(mapped); ok && s.deps.MPC != nil {
s.deps.MPC.SetMode(ctx, mm)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefs remap clobbers planner modes

High Severity

applyPlannerPrefs remaps control mode whenever IsPlannerMode() is true, including planner_self and planner_cheap. Those modes are not part of the battery_export mapping, so saving prefs (even only to change forecast_trust) forces a switch to planner_passive_arbitrage or planner_arbitrage, which can enable grid charging that planner_self previously forbade.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 53e8777. Configure here.

if save != nil {
_ = save(StateKeyBatteryExport, string(export))
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mode confirm can revert trust

Medium Severity

ApplyExportFromMode reads Trust under one lock and writes both fields under another. A concurrent /api/planner/prefs update can land between those calls, so a mode confirm from HA or the app restores the older trust value and drops the user’s slider choice.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 53e8777. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 53e8777ff1

ℹ️ 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".

Comment on lines +77 to +81
if err := s.deps.State.SaveConfig(config.StateKeyForecastTrust, string(trust)); err != nil {
return err
}
if err := s.deps.State.SaveConfig(config.StateKeyBatteryExport, string(export)); err != nil {
return err

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Downgrade before returning on persistence failure

When changing an actively arbitraging site to unknown or not_allowed, any SQLite write failure (for example, a full disk or closed database) returns here before the control mode is changed at line 91. The in-memory preference has already been updated, so /api/status reports that export is forbidden while planner_arbitrage can continue selling from the battery; apply the fail-safe mode transition before this fallible persistence path or roll back the preference and dispatch state together.

Useful? React with 👍 / 👎.

Comment on lines +658 to +660
s.mu.Lock()
s.PVForecastSafetyK = k
request := s.beginReplanLocked(ctx, "safety_k_changed")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Snapshot the safety factor under the planner mutex

When a scheduled or otherwise active replan overlaps a preference POST, this assignment is protected by s.mu, but runReplan later reads s.PVForecastSafetyK without that lock at service.go:1202. The new runtime setter therefore introduces a Go data race; include the factor in replanRequest while holding the mutex, as is already done for Defaults, so each generation uses one synchronized value.

AGENTS.md reference: AGENTS.md:L55-L57

Useful? React with 👍 / 👎.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Left a non-blocking comment: Cursor Bugbot reported 2 unresolved findings, including a high-severity planner-mode remap, so this is not approved. Assigned erikarenhill and HuggeK for human review.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@cursor
cursor Bot requested review from Leitet and erikarenhill August 21, 2026 14:17
Replace Passive/Active as primary strategy buttons with follow-the-forecast
and a battery-export permission. Settings keep house reserve on top and hide
optimizer knobs behind a closed disclosure. Weather no longer asks for array
orientation on the normal path.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 4 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit a54ca30. Configure here.

Comment thread web/app.js
});
primary.replaceChildren(frags.primary);
advanced.replaceChildren(frags.advanced);
primary.hidden = !primary.childElementCount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No return path from manual

High Severity

Filtering all planner_ keys out of the primary mode buttons removes every dashboard control that can enter a planner mode. Manual… still offers idle/self/peak/charge, and /api/planner/prefs only remaps mode when already in a planner mode, so choosing a manual mode leaves the site stuck outside the planner with no web UI way back.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a54ca30. Configure here.

Comment thread web/plan.js
syncPrefsUI();
} finally {
prefsPosting = false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Prefs updates silently dropped

Medium Severity

postPlannerPrefs returns immediately while another prefs POST is in flight, so a second action is discarded with no retry. A common sequence—releasing the trust slider then immediately clicking Allow or Keep off on the upgrade banner—can leave the export confirmation ignored even though the UI looked responsive.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a54ca30. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Left a non-blocking comment: Cursor Bugbot skipped on this head and still has 2 unresolved findings that need human review, so this is not approved. Reviewers are already assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

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