feat(plan): "why" tooltips on the plan table (#4310) - #4311
feat(plan): "why" tooltips on the plan table (#4310)#4311chalfontchubby wants to merge 8 commits into
Conversation
Config plumbing only, for the upcoming per-slot "why" plan tooltip. Off by default so there's zero cost for users who don't enable it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Narrates the plan's existing Chrg/HoldChrg/FrzChrg/Exp/HoldExp/FrzExp decision tree (output.py, publish_html_plan) into a plain-English json_row["reason"] sentence, plus manual-override and no-window "demand" fallback cases. Entirely gated behind plan_why_explanations (off by default) - zero cost when disabled since the branches are skipped outright, and no changes to the optimizer itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…able Wires json_row.reason into renderStateCell() as a title= attribute, via a small local escapeAttr() helper (the state cell isn't guaranteed to share a page with any of the existing escapeHtml() definitions elsewhere in web_helper.py, so this keeps the plan renderer script self-contained). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…hon engine Feedback from the design discussion (github.com/springfall2008/discussions/4310) plus direct guidance: this switch is purely additive/diagnostic and targeted at non-technical users, unlike expert_mode switches which generally change real optimizer/battery behaviour - gating it behind expert_mode would undermine its own purpose, so it's now a standalone switch. Also has fetch_config_options() force prediction_kernel_enable off whenever plan_why_explanations is on. v1's own reason text doesn't touch the prediction hot loop, but wiring the constraint to the switch now means a future richer (cross-window) version of this feature can assume the Python engine without a second flag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
renderStateCell() only added title= to the first <td> - a slot that's both charging/freeze-exporting in the same row (rendered as two adjacent cells via row.split) had no tooltip at all on the second half. Found by hovering the actual rendered output against a real debug.yaml replay. json_row.reason is already a single combined sentence covering both halves, so both cells now carry the same text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… not "what"
The FrzChrg/FrzExp sentences just restated the state in prose
("holds its charge... rather than selling it back") without pointing
to any driver of the decision - a "what" not a "why". Both branches
already have the slot's rate and the user's own cost threshold in
scope (same values used for the cell's colour coding just above), so
surface that comparison instead - it's the concrete number that lets
someone judge for themselves whether the freeze looks reasonable.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… just prose Per discussion springfall2008#4310 feedback: json_row["reason"] stays as the same joined sentence (renderStateCell's title= tooltip is unaffected), but each contributing decision is now also captured as a structured entry in json_row["reason_details"] - a stable reason_code (e.g. "charge_low_rate", "freeze_export_below_threshold", "manual_override") plus the raw values that drove it (rate, threshold, target_percent), kept separate from the rendered text. Lets the sentence and the underlying numbers be consumed/tested independently, without changing the capture mechanism itself (still inline at the same decision points, no re-derivation of optimizer logic). Also fixes a parity gap found while restructuring: manual demand overrides never got a reason note, unlike every other manual-override type - now they do. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
I like the concept, I think the only downside is it could make the HTML and JSON quite a bit bigger. My suggestion would be:
Thoughts? |
|
Reusing comments would prevent embedding values - x>y so doing z That might be fine but it feels quite nice having them there. On all the time implies it is in the c++ path as well which might increase maintenance keeping them in step - I've not really dug into how much it touches both paths yet. Again, not a problem so long as it doesn't add too much to compute load when we start extending it. Not trying to push back - I'm very open to taking advice. |
|
AFAIK the C++ is not impacted as that is just prediction.py |
|
That would work. I don't have it clear in my head yet which bits get accelerated - if there is no impact there and it's just touching one code path that's ideal. It's not adding compute so performance impact should be minimal. Good. |
Per maintainer review on PR springfall2008#4311: - Drop the per-row baked sentence entirely. json_row["reasons"] is now a list of {code, params} only - no text. The templates themselves live once in raw_plan["reason_templates"] (published per response, not per row), and get rendered client-side by substituting each entry's params into its template (renderReasonText() in web_helper.py) - avoids duplicating the same sentence ~96 times per plan. - Drop the plan_why_explanations switch entirely - confirmed with the maintainer this never touches the C++ prediction kernel path (output.py's per-row narration is pure Python regardless), so there's no reason to gate it. Also drops the now-unnecessary forced-Python-engine hook in fetch.py that existed solely to guard against that. - Kept string reason codes (e.g. "charge_low_rate") rather than a numeric enum, per discussion: a string can't silently collide with a different meaning the way a renumbered/reused int could - that safety is structural for strings, but requires active "never renumber" discipline for an int enum. The JSON size difference is negligible either way. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Implemented per your suggestion:
One tweak from your suggestion: kept string reason codes (e.g. Pushed to the PR branch. |
Summary
Adds a plain-English "why" tooltip on each plan-table cell, so non-technical users can self-diagnose common "why did it charge/hold/export here" questions without reading
plan.py- raised in #4310.Shape settled through review from @springfall2008 and @farfromexact on the earlier draft:
publish_html_plan()already decides each slot's Chrg/HoldChrg/FrzChrg/Exp/HoldExp/FrzExp state - no re-derivation of optimizer logic, no changes to the optimizer itself.json_row["reasons"]is a list of{code, params}entries per row - no baked sentence. The actual template text lives once inraw_plan["reason_templates"](published per response, not duplicated per row) and gets rendered client-side by substituting each entry's params into its template (renderReasonText()inweb_helper.py).output.py's per-row narration is pure Python regardless ofprediction_kernel_enable), so there's no switch/opt-in."charge_low_rate") rather than numeric IDs - a string can't silently collide with a different meaning if entries are added over time, where a numeric ID needs active "never renumber, only append" discipline for the same safety.title=tooltip for now - deliberately not mobile/touch-friendly yet. That's planned as a separate follow-up PR (stacked on this branch) rather than in scope here, since the data (reasons/reason_templates) and the rendering are already cleanly decoupled.Test plan
test_plan_why_reason.py- covers every state category (Chrg/HoldChrg/FrzChrg/Exp/HoldExp/FrzExp), both manual-override variants, the demand default, thereasons/reason_templatescode+params contract, the client-sidetitle=wiring (including split cells)../run_pre_commitclean.predbat_debug.yamlreplay (not synthetic fixtures) - tooltips render correctly on live data.🤖 Generated with Claude Code