hierarchy charts: infer sign from root balance

frontend/src/charts/bar.ts

@@ -136,7 +136,7 @@ export class BarChart {
 /** Get the currencies to use for the bar chart. */
 function currencies_to_show(
   data: ParsedBarChartData,
-  ctx: ChartContext,
+  $chartContext: ChartContext,
 ): string[] {
   // Count the usage of each currency in the data.
   const counts = rollup(
@@ -149,7 +149,7 @@ function currencies_to_show(
   );
 
   // Show all operating currencies that are used in the data.
-  const to_show = ctx.currencies.filter((c) => counts.delete(c));
+  const to_show = $chartContext.currencies.filter((c) => counts.delete(c));
 
   // Also add some of the most common other currencies (up to 5 in total)
   to_show.push(
@@ -168,10 +168,10 @@ function currencies_to_show(
 export function bar(
   label: string | null,
   json: unknown,
-  ctx: ChartContext,
+  $chartContext: ChartContext,
 ): Result<BarChart, string> {
   return bar_validator(json).map((parsedData) => {
-    const currencies = currencies_to_show(parsedData, ctx);
+    const currencies = currencies_to_show(parsedData, $chartContext);
 
     const bar_groups = parsedData.map((interval) => ({
       values: currencies.map((currency) => ({
@@ -180,7 +180,7 @@ export function bar(
         budget: interval.budgets[currency] ?? 0,
       })),
       date: interval.date,
-      label: ctx.dateFormat(interval.date),
+      label: $chartContext.dateFormat(interval.date),
       account_balances: interval.account_balances,
     }));
 

frontend/src/charts/hierarchy.ts

@@ -1,3 +1,4 @@
+import { sum } from "d3-array";
 import { hierarchy as d3Hierarchy } from "d3-hierarchy";
 import type { HierarchyNode } from "d3-hierarchy";
 
@@ -19,36 +20,46 @@ import type { ChartContext } from "./context";
 
 /** The data provided with a fava.core.tree.SerialisedTreeNode. */
 export type AccountTreeNode = TreeNode<{
-  account: string;
-  balance: Record<string, number>;
-  balance_children: Record<string, number>;
-  cost: Record<string, number> | null;
-  cost_children: Record<string, number> | null;
-  has_txns: boolean;
+  readonly account: string;
+  readonly balance: Record<string, number>;
+  readonly balance_children: Record<string, number>;
+  readonly cost: Record<string, number> | null;
+  readonly cost_children: Record<string, number> | null;
+  readonly has_txns: boolean;
 }>;
 
 /** The data for a single account in a d3-hierarchy. */
 export type AccountHierarchyDatum = TreeNode<{
-  account: string;
-  balance: Record<string, number>;
-  dummy?: boolean;
+  readonly account: string;
+  readonly balance: Record<string, number>;
+  readonly dummy: boolean;
+}>;
+
+export type AccountHierarchyInputDatum = TreeNode<{
+  readonly account: string;
+  readonly balance: Record<string, number>;
 }>;
 
 /** A d3-hierarchy node for an account. */
 export type AccountHierarchyNode = HierarchyNode<AccountHierarchyDatum>;
 
 /**
- * Add internal nodes as fake leaf nodes to their own children.
+ * Add internal nodes as dummy leaf nodes to their own children.
  *
  * In the treemap, we only render leaf nodes, so for accounts that have both
  * children and a balance, we want to duplicate them as leaf nodes.
  */
-function addInternalNodesAsLeaves(node: AccountHierarchyDatum): void {
-  if (node.children.length) {
-    node.children.forEach(addInternalNodesAsLeaves);
-    node.children.push({ ...node, children: [], dummy: true });
-    node.balance = {};
+export function addInternalNodesAsLeaves({
+  account,
+  balance,
+  children,
+}: AccountHierarchyInputDatum): AccountHierarchyDatum {
+  if (children.length) {
+    const c = children.map(addInternalNodesAsLeaves);
+    c.push({ account, balance, children: [], dummy: true });
+    return { account, balance: {}, children: c, dummy: false };
   }
+  return { account, balance, children: [], dummy: false };
 }
 
 export class HierarchyChart {
@@ -70,30 +81,41 @@ export const account_hierarchy_validator: Validator<AccountTreeNode> = object({
   has_txns: boolean,
 });
 
-const hierarchy_validator = object({
-  root: account_hierarchy_validator,
-  modifier: number,
-});
+export function hierarchy_from_parsed_data(
+  label: string | null,
+  data: AccountHierarchyInputDatum,
+  { currencies }: ChartContext,
+): HierarchyChart {
+  const root = addInternalNodesAsLeaves(data);
+  return new HierarchyChart(
+    label,
+    new Map(
+      currencies
+        .map((currency) => {
+          const r = d3Hierarchy<AccountHierarchyDatum>(root);
+          const root_balance = sum(
+            r.descendants(),
+            (n) => n.data.balance[currency] ?? 0,
+          );
+          // depending on the balance for this currency in the root,
+          // build the tree either for all positive values or all negative values
+          const sign = root_balance ? Math.sign(root_balance) : 1;
+          r.sum(
+            (d) => sign * Math.max(sign * (d.balance[currency] ?? 0), 0),
+          ).sort((a, b) => sign * ((b.value ?? 0) - (a.value ?? 0)));
+          return [currency, r] as const;
+        })
+        .filter(([, h]) => h.value),
+    ),
+  );
+}
 
 export function hierarchy(
   label: string | null,
   json: unknown,
-  { currencies }: ChartContext,
+  $chartContext: ChartContext,
 ): Result<HierarchyChart, string> {
-  return hierarchy_validator(json).map((value) => {
-    const { root, modifier } = value;
-    addInternalNodesAsLeaves(root);
-    const data = new Map<string, AccountHierarchyNode>();
-
-    currencies.forEach((currency) => {
-      const currencyHierarchy = d3Hierarchy<AccountHierarchyDatum>(root)
-        .sum((d) => Math.max((d.balance[currency] ?? 0) * modifier, 0))
-        .sort((a, b) => (b.value ?? 0) - (a.value ?? 0));
-      if (currencyHierarchy.value) {
-        data.set(currency, currencyHierarchy);
-      }
-    });
-
-    return new HierarchyChart(label, data);
-  });
+  return account_hierarchy_validator(json).map((r) =>
+    hierarchy_from_parsed_data(label, r, $chartContext),
+  );
 }

frontend/src/charts/index.ts

@@ -19,7 +19,11 @@ import type { ScatterPlot } from "./scatterplot";
 
 const parsers: Record<
   string,
-  (label: string, json: unknown, ctx: ChartContext) => Result<FavaChart, string>
+  (
+    label: string,
+    json: unknown,
+    $chartContext: ChartContext,
+  ) => Result<FavaChart, string>
 > = {
   balances,
   bar,
@@ -35,14 +39,14 @@ const chart_data_validator = array(
 
 export function parseChartData(
   data: unknown,
-  ctx: ChartContext,
+  $chartContext: ChartContext,
 ): Result<FavaChart[], string> {
   return chart_data_validator(data).map((chartData) => {
     const result: FavaChart[] = [];
     chartData.forEach((chart) => {
       const parser = parsers[chart.type];
       if (parser) {
-        const r = parser(chart.label, chart.data, ctx);
+        const r = parser(chart.label, chart.data, $chartContext);
         if (r.is_ok) {
           result.push(r.value);
         }

frontend/src/charts/query-charts.ts

@@ -1,12 +1,10 @@
-import { hierarchy as d3Hierarchy } from "d3-hierarchy";
-
 import type { Result } from "../lib/result";
 import { stratify } from "../lib/tree";
 import { array, number, object, record, string } from "../lib/validation";
 
 import type { ChartContext } from "./context";
-import type { AccountHierarchyDatum, AccountHierarchyNode } from "./hierarchy";
-import { HierarchyChart } from "./hierarchy";
+import type { HierarchyChart } from "./hierarchy";
+import { hierarchy_from_parsed_data } from "./hierarchy";
 import type { LineChart } from "./line";
 import { balances } from "./line";
 
@@ -16,29 +14,18 @@ const grouped_chart_validator = array(
 
 export function parseGroupedQueryChart(
   json: unknown,
-  { currencies }: ChartContext,
+  $chartContext: ChartContext,
 ): Result<HierarchyChart, string> {
   return grouped_chart_validator(json)
     .map_err(() => "No grouped query data")
     .map((grouped) => {
-      const root: AccountHierarchyDatum = stratify(
+      const root = stratify(
         grouped,
         (d) => d.group,
         (account, d) => ({ account, balance: d?.balance ?? {} }),
       );
       root.account = "(root)";
-
-      const data = new Map<string, AccountHierarchyNode>();
-      currencies.forEach((currency) => {
-        const currencyHierarchy = d3Hierarchy(root)
-          .sum((d) => d.balance[currency] ?? 0)
-          .sort((a, b) => (b.value ?? 0) - (a.value ?? 0));
-        if (currencyHierarchy.value !== undefined) {
-          data.set(currency, currencyHierarchy);
-        }
-      });
-
-      return new HierarchyChart(null, data);
+      return hierarchy_from_parsed_data(null, root, $chartContext);
     });
 }
 
@@ -48,10 +35,10 @@ export function parseGroupedQueryChart(
  */
 export function parseQueryChart(
   json: unknown,
-  ctx: ChartContext,
+  $chartContext: ChartContext,
 ): Result<HierarchyChart | LineChart, string> {
   return (
-    parseGroupedQueryChart(json, ctx)
+    parseGroupedQueryChart(json, $chartContext)
       // Try balances chart if the grouped chart parse
       .or_else(() => balances(null, json))
       .map_err(() => "No query chart found.")

frontend/src/lib/tree.ts

@@ -6,7 +6,7 @@ import { parent } from "./account";
  * The only base property this has is `.children`, all others are
  * passed in via the generic parameter.
  */
-export type TreeNode<S> = S & { children: TreeNode<S>[] };
+export type TreeNode<S> = S & { readonly children: TreeNode<S>[] };
 
 /**
  * Generate an account tree from an array.

frontend/test/charts.test.ts

@@ -97,13 +97,12 @@ test("handle data for query charts", () => {
   const ctx = { currencies: ["EUR"], dateFormat: () => "DATE" };
   const d = [{ group: "Assets:Cash", balance: { EUR: 10 } }];
   const { data } = parseGroupedQueryChart(d, ctx).unwrap();
-  assert.is(data.get("EUR")?.value, 10);
+  const eur_hierarchy = data.get("EUR");
+  assert.ok(eur_hierarchy);
+  assert.is(eur_hierarchy.value, 10);
   assert.equal(
-    data
-      .get("EUR")
-      ?.descendants()
-      .map((n) => n.data.account),
-    ["(root)", "Assets", "Assets:Cash"],
+    eur_hierarchy.descendants().map((n) => n.data.account),
+    ["(root)", "Assets", "(root)", "Assets:Cash", "Assets"],
   );
 });
 

src/fava/internal_api.py

@@ -15,7 +15,6 @@
 from flask import url_for
 from flask_babel import gettext  # type: ignore[import]
 
-from fava.beans.account import root
 from fava.context import g
 from fava.util.excel import HAVE_EXCEL
 
@@ -160,28 +159,16 @@ def _chart_hierarchy(
     end_date: date | None = None,
     label: str | None = None,
 ) -> ChartData:
-    modifier = (
-        +1
-        if root(account_name)
-        in (
-            g.ledger.options["name_assets"],
-            g.ledger.options["name_expenses"],
-        )
-        else -1
-    )
     return ChartData(
         "hierarchy",
         label or account_name,
-        {
-            "modifier": modifier,
-            "root": g.ledger.charts.hierarchy(
-                g.filtered,
-                account_name,
-                g.conversion,
-                begin_date,
-                end_date or g.filtered.end_date,
-            ),
-        },
+        g.ledger.charts.hierarchy(
+            g.filtered,
+            account_name,
+            g.conversion,
+            begin_date,
+            end_date or g.filtered.end_date,
+        ),
     )