carlos / platform Public

console: fix bill_services clobber + non-representable zero selection

Fix round on Task 13 (/fleet/economics), two Important findings:

Finding 1: readScenarioFromDOM wrote sc.bill_services = [] whenever no
bill checkboxes existed on screen (no bill for this period) — since the
manual bill-entry form shares #econ-live's delegated input/change
listener, one keystroke there fired recompute() and clobbered a
reconciliation selection saved against a DIFFERENT period's bill. Fixed
by gating the whole bill_services branch on boxes.length > 0; with no
checkboxes, the cloned base scenario's bill_services passes through
untouched.

Finding 2: unchecking every "counts as CARLOS" box serialized [], which
reads back as "count everything" — the sign flips silently on reload,
because empty already means all and there is no tri-state for "count
nothing". Per the controller's ruling, the save field now keeps the last
selection that had at least one box checked (lastNonEmptyBillServices,
seeded from the loaded scenario) instead of serializing the all-unchecked
state, and an inline note (#bill-services-zero-note) tells the operator
the live /bin/bash view isn't what gets saved.

Also dropped "Project" from the header comment's transliteration claim —
compute.go's Project isn't used by this page.

internal/console/economicspage_test.go: TestEconomicsPageBillServicesGuardShape
greps the served page for both fix shapes (and the absence of the old
ungated ternary) — a structural stand-in for a browser test, since this
suite has no headless DOM.

🤖 Generated with Claude Code
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell cb00fee47235352bf1b29f93cffb7910db61be5a parent 88a71f3
2 files changed, +132 −8
  • internal/console/economicspage_test.go +61 −0
  • internal/console/templates/economics.html +71 −8
diff --git a/internal/console/economicspage_test.go b/internal/console/economicspage_test.go
index 8f6a4436a..ef035b6ea 100644
--- a/internal/console/economicspage_test.go
+++ b/internal/console/economicspage_test.go
@@ -170,3 +170,64 @@ func TestEconomicsDefaultScenarioValid(t *testing.T) {
t.Fatalf("DefaultScenario: %v", err)
}
}
+
+// TestEconomicsPageBillServicesGuardShape is a structural stand-in for a
+// browser test (none exists in this suite — no headless DOM): it greps the
+// SERVED page for the exact guard shapes the fix-round review demanded, so
+// a future edit that reintroduces either bug fails a test instead of only
+// a manual re-review.
+//
+// Finding 1 (economics.html, fix round 1): the original one-line ternary
+// `(boxes.length > 0 && checked.length < boxes.length) ? checked : []`
+// wrote [] whenever no bill checkboxes existed on screen — clobbering the
+// base scenario's saved bill_services from an unrelated keystroke in the
+// manual-bill-entry form sharing #econ-live. The fix wraps the whole
+// bill_services branch in `if (boxes.length > 0)`, leaving sc.bill_services
+// untouched when there are no checkboxes at all.
+//
+// Finding 2: an all-unchecked selection is not a representable
+// reconciliation state (the empty list already means "count everything").
+// The fix keeps a lastNonEmptyBillServices variable and an inline
+// #bill-services-zero-note the script shows exactly when every checkbox is
+// unchecked.
+func TestEconomicsPageBillServicesGuardShape(t *testing.T) {
+ fx := newConsoleFixture(t)
+ seedEconomicsGolden(t, fx)
+ c := signInPaul(t, fx)
+ if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil {
+ t.Fatal(err)
+ }
+ status, _, body := get(t, c, fx.ts.URL+"/fleet/economics")
+ if status != 200 {
+ t.Fatalf("/fleet/economics = %d, want 200:\n%s", status, body)
+ }
+
+ old := "(boxes.length > 0 && checked.length < boxes.length) ? checked : []"
+ if strings.Contains(body, old) {
+ t.Fatalf("/fleet/economics still contains the pre-fix ungated ternary %q — Finding 1 regressed", old)
+ }
+ for _, want := range []string{
+ // Finding 1: the bill_services branch is gated on boxes existing,
+ // so a page with no bill on screen never touches sc.bill_services.
+ `if (boxes.length > 0) {`,
+ // Finding 2: the last-non-empty-selection fallback and its visible
+ // note both exist, wired to the all-unchecked branch.
+ "lastNonEmptyBillServices",
+ `id="bill-services-zero-note"`,
+ "sc.bill_services = lastNonEmptyBillServices !== null ? lastNonEmptyBillServices : sc.bill_services;",
+ "setBillServicesZeroNote(true)",
+ "setBillServicesZeroNote(false)",
+ } {
+ if !strings.Contains(body, want) {
+ t.Fatalf("/fleet/economics missing guard-shape string %q", want)
+ }
+ }
+
+ // The header comment claiming a "Project" transliteration was dropped
+ // (the script renders no projections) — check the doc-comment line
+ // itself, not the word in isolation (which could legitimately appear
+ // elsewhere, e.g. an English sentence using "project" as a verb).
+ if strings.Contains(body, "AllocateBox/PriceAccount/Project") {
+ t.Fatalf("/fleet/economics still claims a Project transliteration in its header comment")
+ }
+}
diff --git a/internal/console/templates/economics.html b/internal/console/templates/economics.html
index e781220eb..399fef64e 100644
--- a/internal/console/templates/economics.html
+++ b/internal/console/templates/economics.html
@@ -3,7 +3,7 @@
is already priced under the active scenario (economicsview.go) — the
ONE inline <script> at the foot only RE-computes in place when an
operator tweaks a knob, transliterating
- internal/economics/compute.go's AllocateBox/PriceAccount/Project
+ internal/economics/compute.go's AllocateBox/PriceAccount
function-for-function (same names, JS-cased) so drift between the two
is reviewable at a glance. With JS off, every number above stays
exactly what the server rendered, and the knob drawer's only working
@@ -134,7 +134,22 @@
reconciliation. Once anything is unchecked, bill_services carries
exactly the checked set. billServiceChecked (server.go) and the inline
script's JS twin of the same name are the ONE place this rule is
- encoded — nowhere else re-derives it. */}}
+ encoded — nowhere else re-derives it.
+
+ Two corollaries the inline script's readScenarioFromDOM enforces
+ (controller ruling, fix round 1):
+ - Empty-means-all is only a READING of an empty list, never a
+ WRITING instruction: when this period's page shows no checkboxes
+ at all (no bill on screen), the save field must pass the base
+ scenario's bill_services through untouched, not overwrite it to
+ [] — the manual bill-entry form shares #econ-live's delegated
+ listener, so a keystroke there must not clobber a reconciliation
+ selection made on a DIFFERENT period's checkboxes.
+ - "Count zero services" is not a meaningful reconciliation state
+ (there is no tri-state to invent between "all" and "some"): when
+ every checkbox is unchecked, the hidden save field keeps the last
+ selection that had at least one box checked, and the drawer shows
+ a note that the all-unchecked view is live-only, not saveable. */}}
{{$billChecked := sumBillServicesChecked $curBill.Services .Economics.Scenario.BillServices}}
{{$delta := sub $billChecked $modelTotal}}
<div class="tablewrap">
@@ -250,6 +265,11 @@
</div>
<label><input type="checkbox" name="activate" value="1"> Activate</label>
<button type="submit">Save scenario</button>
+ {{/* Hidden by default; the inline script shows it (display:"") exactly
+ when every "counts as CARLOS" checkbox is unchecked — the state the
+ save field refuses to persist (see the template comment by the bill
+ table). */}}
+ <p class="small error" id="bill-services-zero-note" style="display:none">At least one service must count — unchecking all shows a zero model line but is not saveable as a selection.</p>
</form>
</details>
</div>
@@ -267,6 +287,20 @@
var payload;
try { payload = JSON.parse(dataEl.textContent); } catch (e) { return; }
+ // lastNonEmptyBillServices remembers the most recent "counts as CARLOS"
+ // selection that had at least one box checked, seeded from the scenario
+ // the page loaded with — readScenarioFromDOM's zero-checked branch reads
+ // this instead of ever serializing an empty (== "count everything",
+ // the opposite meaning) selection. null until either the loaded scenario
+ // or this session has seen a non-empty selection.
+ var lastNonEmptyBillServices = (payload.scenario.bill_services && payload.scenario.bill_services.length > 0)
+ ? payload.scenario.bill_services
+ : null;
+ function setBillServicesZeroNote(show) {
+ var el = document.getElementById("bill-services-zero-note");
+ if (el) { el.style.display = show ? "" : "none"; }
+ }
+
// HoursPerMonth mirrors internal/economics/compute.go's exported constant
// of the same name — the proration every hour-denominated rate uses to
// read against a GB-month figure.
@@ -470,13 +504,42 @@
sc.free_tier.mem_gb_hours = numVal("knob-ft-mem-gb-hours");
sc.free_tier.storage_gb = numVal("knob-ft-storage-gb");
+ // See the template comment by the bill table (the two corollaries
+ // block) for both rules enforced below.
var boxes = document.querySelectorAll(".bill-service-checkbox");
- var checked = [];
- boxes.forEach(function (el) { if (el.checked) { checked.push(el.getAttribute("data-service")); } });
- // Empty means "every service counts" (billServiceChecked's default) —
- // so leaving every box checked writes [] back, not an equivalent,
- // brittle explicit list of every service name on the bill.
- sc.bill_services = (boxes.length > 0 && checked.length < boxes.length) ? checked : [];
+ if (boxes.length > 0) {
+ var checked = [];
+ boxes.forEach(function (el) { if (el.checked) { checked.push(el.getAttribute("data-service")); } });
+ if (checked.length > 0) {
+ // Empty means "every service counts" (billServiceChecked's
+ // default) — so leaving every box checked writes [] back, not an
+ // equivalent, brittle explicit list of every service name on the
+ // bill.
+ sc.bill_services = checked.length < boxes.length ? checked : [];
+ lastNonEmptyBillServices = sc.bill_services;
+ setBillServicesZeroNote(false);
+ } else {
+ // All unchecked: "count zero services" is not a representable
+ // reconciliation state (empty already means "count everything",
+ // the opposite meaning). Keep the save field at the last
+ // selection that had at least one box checked this session, or —
+ // if the operator hasn't touched a checkbox yet — the base
+ // scenario's own saved list, either way never []. The live
+ // model/bill numbers on screen may still show the all-unchecked
+ // $0 total; only the SAVED state declines to represent it, which
+ // is why the note below exists.
+ sc.bill_services = lastNonEmptyBillServices !== null ? lastNonEmptyBillServices : sc.bill_services;
+ setBillServicesZeroNote(true);
+ }
+ }
+ // boxes.length === 0: no bill is on screen for this period, so there
+ // is nothing to derive a selection from — sc.bill_services stays
+ // exactly what the clone of base already carries, untouched. Without
+ // this early-out, a keystroke in the unrelated manual-bill-entry form
+ // (inside the same #econ-live listener scope) would fire recompute()
+ // and clobber a reconciliation selection made against a DIFFERENT
+ // period's bill into [] — the fix-round finding this whole branch
+ // exists for.
return sc;
}