Skip to content

Feature/cooling electricity ground truth fixes - #109

Merged
jravani merged 9 commits into
mainfrom
feature/cooling-electricity-ground-truth-fixes
Aug 3, 2026
Merged

jravani merged 9 commits into
mainfrom
feature/cooling-electricity-ground-truth-fixes

Conversation

@jravani

@jravani jravani commented Aug 1, 2026

Copy link
Copy Markdown
Member

No description provided.

jravani added 6 commits August 1, 2026 21:58
solver.compute_cooling was never sent (BuEM defaults it false), so the
response never carried a cooling field at all. Send it true by default
and relabel the "Hot Water" UI slot (Droplets icon, chart legend, cards)
to "Cooling" (Snowflake) — hot water itself had no real calculation
behind it, only ever a design placeholder.

Fixes #102
BuEM's raw cooling timeseries is negative (its solver shares one signed
heating/cooling axis) — negate it at the API boundary so it reads as a
plain positive "energy needed" figure everywhere downstream. Also swap
the chart's electricity/cooling line colors, which were backwards
relative to the icons already used in the summary cards.

Fixes #103
…/download

The Combined view had no visual reference for what's typical across the
period, so add a dashed average line labelled with its value. Also drop
the mode==='expert' gate on the Upload/Download load-profile controls —
the exported format is plain CSV in a zip, safe and useful for Basic-mode
users too.

Fixes #104
The sidebar's headline "Heating" figure always showed ignis's live
estimate whenever ignis had any result — which is nearly always, since
ignis recalculates on every field edit independent of Recalculate.
BuEM's fresh result from Recalculate only ever fed the delta badge,
never became the headline. Track confirmation state (isHeatingConfirmed):
true right after Recalculate, false the moment ignis produces a new live
result. Show BuEM's result when confirmed, ignis's estimate otherwise.

Also replace the "▲/▼ N%" badge with a plain-language sentence, e.g.
"26% lower than the last full simulation (24,955 kWh)".

Fixes #105
Was rendered on its own line below the total, disconnected from it.
Moves "(38.4 kWh/m²·a)" directly in front of the total figure on the
same line instead.

Fixes #106
… kWh/year

Uploading a load profile only ever fed the chart — it never reached the
"Annual energy demand" totals, so users with real measured data couldn't
compare model output against it. When a profile is uploaded it now
becomes the headline for all three energy totals (electricity, heating,
cooling), with ignis's live estimate / BuEM's confirmed result shown as
a comparison against it instead. Fixing this needed pickUpdatedRows() to
replace pickAnyRows(): the old helper grabbed the model's pre-existing
hourly data instead of whatever resolution the file just populated.

Also move the "estimated"/"user defined" source tag from next to the
value to next to the label (generalized from heating-only to all three
energy types), and change the annual totals unit from "kWh" to
"kWh/year".

Fixes #107
Copilot AI review requested due to automatic review settings August 1, 2026 21:07
@jravani

jravani commented Aug 1, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves how annual energy totals (heating/electricity/cooling) are sourced and presented in the Building Configurator by introducing explicit “source” metadata (BuEM vs. ignis estimate vs. user-uploaded ground truth), fixing BuEM cooling sign handling, and plumbing uploaded load profiles through to override headline totals.

Changes:

  • Add “ground truth” flow for uploaded load profiles and use it as the highest-priority source for displayed annual totals.
  • Correct BuEM cooling timeseries sign and enable cooling computation by default in the BuEM request payload.
  • Update overview UI to label totals with their source and show a plain-language comparison note instead of the old heating-only delta badge.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/app/lib/loadProfile.ts Adds EnergySource/expanded EnergyTotals metadata and introduces pickUpdatedRows to identify newly uploaded series.
src/app/lib/buemApi.ts Flips BuEM cooling timeseries sign so downstream consumers see positive cooling demand.
src/app/lib/buemAdapter.ts Enables compute_cooling by default and preserves it on import.
src/app/components/BuildingConfigurator/shared/ui.tsx Replaces heating-only delta badge with SourceTag and EnergyComparisonNote.
src/app/components/BuildingConfigurator/overview/useLoadProfileState.ts Emits uploaded rows as “ground truth” via onGroundTruthChange.
src/app/components/BuildingConfigurator/overview/LoadProfileViewer.tsx Exposes upload/download in more modes, relabels “hotwater” as cooling, and adds a combined-series average reference line.
src/app/components/BuildingConfigurator/overview/EnergyEnvelopeColumn.tsx Plumbs onGroundTruthChange through to the viewer.
src/app/components/BuildingConfigurator/overview/BuildingSnapshotAside.tsx Displays per-row source tags and comparison notes for heating/electricity/cooling.
src/app/components/BuildingConfigurator/BuildingConfigurator.tsx Adds ground-truth + “confirmed heating” state and centralizes total-resolution logic in resolveDisplayEnergyTotals.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app/components/BuildingConfigurator/BuildingConfigurator.tsx
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 1, 2026 21:12

Copilot AI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved and pushed. Merge conflicts are now addressed in commit 7c9c28a (with branch sync in 9174fc5).

Comment on lines +13 to +18
import {
elementToGroup,
isElementEditable,
normalizeElementRecord,
faceFromAzimuth,
} from './configure/model/buildingElements';

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/app/components/BuildingConfigurator/shared/ui.tsx:99

  • SourceTag re-declares the energy source union type instead of reusing the shared EnergySource type from lib/loadProfile. Keeping a single source of truth prevents accidental drift if the union changes.
export function SourceTag({ source }: { source?: 'ignis' | 'buem' | 'user' }) {

src/app/lib/buemApi.ts:145

  • Cooling is still being mapped into the LoadDataPoint.hotwater field. Now that the UI and exports treat this series as "Cooling", keeping the underlying property named "hotwater" is misleading and makes future work (e.g. adding real DHW) error-prone.
        // LoadDataPoint has no cooling field. Reusing "hotwater" for BuEM's
        // cooling output matches the existing thermalSummary-based fallback
        // in BuildingConfigurator.tsx's computeEnergyTotals — not a
        // physically accurate label, but the established convention.
        // BuEM reports cooling as negative (its solver shares one signed
        // Q_HC axis: positive = heat added, negative = heat removed) — flip
        // it here so every downstream reader (chart, CSV export, totals)
        // sees a plain positive "energy needed for cooling" figure.
        hotwater: -(ts.cooling?.[i] ?? 0),

Copilot AI review requested due to automatic review settings August 1, 2026 21:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/app/components/BuildingConfigurator/BuildingConfigurator.tsx:1330

  • This icon-only close button (×) has no accessible name. Add an aria-label so screen readers can announce what it does.
                      <button
                        type="button"
                        onClick={() => setPvInvalidated(false)}
                        className="shrink-0 cursor-pointer text-sm leading-none text-amber-600"
                      >×</button>

src/app/lib/buemApi.ts:145

  • Cooling timeseries values are flipped to positive, but thermalSummary.coolingKwh is still taken directly from the API response. If BuEM cooling totals share the same signed convention as the timeseries (as noted above), the summary will remain negative and disagree with chart/totals when the UI falls back to thermalSummary.
        // BuEM reports cooling as negative (its solver shares one signed
        // Q_HC axis: positive = heat added, negative = heat removed) — flip
        // it here so every downstream reader (chart, CSV export, totals)
        // sees a plain positive "energy needed for cooling" figure.
        hotwater: -(ts.cooling?.[i] ?? 0),

src/app/components/BuildingConfigurator/shared/ui.tsx:99

  • SourceTag duplicates the EnergySource union instead of reusing the central type from lib/loadProfile.ts, which risks drift if sources change (and makes refactors harder).
export function SourceTag({ source }: { source?: 'ignis' | 'buem' | 'user' }) {

src/app/components/BuildingConfigurator/BuildingConfigurator.tsx:1316

  • This icon-only close button (×) has no accessible name. Add an aria-label so screen readers can announce what it does.

This issue also appears on line 1326 of the same file.

                      <button
                        type="button"
                        onClick={() => setUploadError(null)}
                        className="shrink-0 cursor-pointer text-sm leading-none text-destructive"
                      >×</button>

src/app/components/BuildingConfigurator/BuildingConfigurator.tsx:288

  • HeaderBtn renders an icon-only and relies on the title attribute for labeling. title isn't a reliable accessible name for screen readers; add aria-label (ideally matching tooltip).
      onClick={onClick}
      title={tooltip}
      className="size-7 flex items-center justify-center rounded-md cursor-pointer text-muted-foreground hover:bg-muted transition-colors duration-100 shrink-0 [&_svg]:size-4"

@jravani
jravani merged commit e6d6d5a into main Aug 3, 2026
2 checks passed
@jravani
jravani deleted the feature/cooling-electricity-ground-truth-fixes branch August 4, 2026 00:19
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