fix(reporting): fair "Best" tagging on the scenario compare screen

Three fixes from feedback on Compare scenarios:

- Ties no longer default to the leftmost column. pickBestIndex becomes
  pickBestIndices, returning every column that holds the joint-best value,
  so tied columns are all marked Best. Average EPC is compared on the
  rounded SAP shown, so a visible "71 = 71" reads as a real tie.
- CO2 saved /yr renders to one decimal (shared formatTonnes), so two
  scenarios that both rounded to "2 t" but differ — the reason one is
  Best — are visibly distinct.
- Funding secured no longer crowns £0 as Best (new hideBestAtZero flag);
  a zero isn't an achievement. Fields where 0 is a genuine best (Homes
  below C after) are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Khalim Conn-Kowlessar 2026-07-27 11:28:00 +00:00
parent ce417ef16b
commit dcb301a931
3 changed files with 62 additions and 26 deletions

View file

@ -10,12 +10,13 @@ import {
} from "@/app/shadcn_components/ui/popover";
import { sapToEpc } from "@/app/utils";
import {
pickBestIndex,
pickBestIndices,
countBelowBand,
amountSaved,
capitalOutlay,
costPerSapPoint,
costPerCarbonSaved,
formatTonnes,
} from "@/lib/reporting/model";
import { EpcChip, moneyFull } from "../components/primitives";
@ -41,6 +42,13 @@ interface RowSpec {
direction: "lower" | "higher" | null;
value: (m: ScenarioMetrics, b: Baseline) => number | null;
render: (v: number | null) => React.ReactNode;
/**
* Suppress the "Best" tag when the winning value is 0 for fields where a
* zero isn't an achievement (e.g. Funding secured: £0 means none was secured,
* so crowning it "Best" is meaningless). Fields where 0 *is* a genuine best
* (Homes below C after) leave this off.
*/
hideBestAtZero?: boolean;
}
interface ScenarioMetrics {
@ -61,9 +69,12 @@ interface ScenarioMetrics {
const ROWS: RowSpec[] = [
{
group: "Outcome",
// Compare on the rounded SAP the cell shows, so two columns that both
// display e.g. 71 register as a genuine tie (both marked Best) rather than
// the tag hanging on a sub-point difference the reader can't see.
label: "Average EPC after",
direction: "higher",
value: (m) => Number(m.avg_sap),
value: (m) => Math.round(Number(m.avg_sap)),
render: (v) =>
v === null ? "—" : (
<span className="inline-flex items-center gap-1.5">
@ -87,10 +98,12 @@ const ROWS: RowSpec[] = [
},
{
group: "Outcome",
// One decimal (shared formatTonnes) so two scenarios that both round to
// "2 t" but differ — the reason one is Best — are visibly distinct.
label: "CO₂ saved /yr",
direction: "higher",
value: (m, b) => amountSaved(b.totalCarbon, m.total_carbon),
render: (v) => (v === null ? "—" : `${Math.round(v)} t`),
render: (v) => (v === null ? "—" : `${formatTonnes(v)} t`),
},
{
group: "Outcome",
@ -112,6 +125,7 @@ const ROWS: RowSpec[] = [
direction: "higher",
value: (m) => m.total_funding,
render: (v) => (v === null ? "—" : moneyFull(v)),
hideBestAtZero: true,
},
{
group: "Investment",
@ -351,8 +365,16 @@ function GroupRows({
);
const best =
row.direction === null
? null
: pickBestIndex(scenarioValues, row.direction);
? new Set<number>()
: pickBestIndices(scenarioValues, row.direction);
// A zero winner isn't an achievement on some fields (e.g. no funding
// secured) — drop the tag there rather than crowning £0.
if (row.hideBestAtZero) {
const anyWinner = best.values().next().value;
if (anyWinner !== undefined && (scenarioValues[anyWinner] ?? 0) === 0) {
best.clear();
}
}
// "before" cell: baseline has no scenario metric, so most rows show —.
const baselineCell =
row.label === "Average EPC after"
@ -370,10 +392,10 @@ function GroupRows({
<td
key={i}
className={`px-4 py-2.5 text-right tabular-nums ${
best === i ? "font-bold text-[#0c6b4a]" : "text-gray-900"
best.has(i) ? "font-bold text-[#0c6b4a]" : "text-gray-900"
}`}
>
{best === i && (
{best.has(i) && (
<span className="mr-1.5 inline-flex items-center rounded bg-[#e9f4ef] px-1.5 py-0.5 align-middle text-[0.6rem] font-bold uppercase tracking-wide text-[#0c6b4a]">
Best
</span>

View file

@ -11,7 +11,7 @@ import {
deriveLedgerView,
formatTonnes,
isCompliantBeyondWindow,
pickBestIndex,
pickBestIndices,
selectGoalCallout,
shapeKpiDelta,
toBandCounts,
@ -281,24 +281,32 @@ describe("classifyBandMovement", () => {
});
});
describe("pickBestIndex", () => {
describe("pickBestIndices", () => {
// Compare view (Screen D): mark the best value in each row without
// crowning an overall winner. Nulls (baseline / missing) never win.
it("picks the lowest for a lower-is-better row", () => {
expect(pickBestIndex([null, 50, 74, 103], "lower")).toBe(1);
expect(pickBestIndices([null, 50, 74, 103], "lower")).toEqual(new Set([1]));
});
it("picks the highest for a higher-is-better row", () => {
expect(pickBestIndex([null, 312, 368, 219], "higher")).toBe(2);
expect(pickBestIndices([null, 312, 368, 219], "higher")).toEqual(
new Set([2]),
);
});
it("returns null when every comparable value is missing", () => {
expect(pickBestIndex([null, null], "lower")).toBeNull();
it("marks every column that ties on the best value", () => {
expect(pickBestIndices([null, 71, 71, 68], "higher")).toEqual(
new Set([1, 2]),
);
});
it("returns an empty set when every comparable value is missing", () => {
expect(pickBestIndices([null, null], "lower")).toEqual(new Set());
});
it("ignores nulls rather than treating them as zero", () => {
expect(pickBestIndex([null, null, 5], "lower")).toBe(2);
expect(pickBestIndices([null, null, 5], "lower")).toEqual(new Set([2]));
});
});

View file

@ -269,24 +269,30 @@ const GOAL_DIMENSIONS: Partial<Record<string, GoalDimension>> = {
};
/**
* Index of the best value in a compare row (Screen D). Nulls baseline
* columns and missing data never win. Returns null when nothing is
* comparable, so no cell is marked.
* Indices of the best value in a compare row (Screen D) *every* column that
* holds the joint-best value, so a tie marks all of them rather than silently
* crowning the leftmost. Nulls baseline columns and missing data never win.
* Returns an empty set when nothing is comparable, so no cell is marked.
*
* Ties are decided by exact equality: a caller that rounds for display (e.g.
* Average EPC whole SAP) should round the compared value too, so a tie the
* reader can see is a tie the tag agrees with.
*/
export function pickBestIndex(
export function pickBestIndices(
values: (number | null)[],
direction: "lower" | "higher",
): number | null {
let bestIndex: number | null = null;
): Set<number> {
let best = direction === "lower" ? Infinity : -Infinity;
for (const v of values) {
if (v === null) continue;
if (direction === "lower" ? v < best : v > best) best = v;
}
const winners = new Set<number>();
if (!Number.isFinite(best)) return winners; // every comparable value was null
values.forEach((v, i) => {
if (v === null) return;
if (direction === "lower" ? v < best : v > best) {
best = v;
bestIndex = i;
}
if (v === best) winners.add(i);
});
return bestIndex;
return winners;
}
/** Collapses baseline band rows (actual + estimated) into a plain band→count map. */