holdings: value positions at the time-filter end date

CHANGES

@@ -4,6 +4,8 @@ Changelog
 Unreleased
 ----------
 
+Holdings reports now value positions at the time-filter end date, matching
+account reports, instead of using later prices from outside the filter.
 A Finnish translation was added. The date filter parser was rewritten from
 a regex-based implementation to a proper lexer/parser, fixing several edge-case
 bugs. As part of this, the undocumented `to` range separator (e.g. `2010 to

frontend/src/api/index.ts

@@ -61,6 +61,7 @@ type GetEndpoint =
   | "events"
   | "extract"
   | "help"
+  | "holdings"
   | "imports"
   | "income_statement"
   | "journal_page"
@@ -89,6 +90,7 @@ type ApiEndpoint = DeleteEndpoint | GetEndpoint | PutEndpoint;
 type ApiParams = Partial<{
   a: string;
   account: string;
+  aggregation_key: string;
   conversion: string;
   entry_hash: string;
   filename: string;
@@ -272,6 +274,14 @@ export const get_help = define_endpoint(
   object({ html: string, pages: array(tuple(string, string)) }),
   ["page_slug"],
 );
+export const get_holdings = define_endpoint(
+  "holdings",
+  object({
+    query_string: string,
+    query_result_table: query_validator,
+  }),
+  [...filters, "aggregation_key"],
+);
 export const get_imports = define_paramless_endpoint(
   "imports",
   importable_files_validator,

frontend/src/reports/holdings/index.ts

@@ -1,4 +1,4 @@
-import { get_query } from "../../api/index.ts";
+import { get_holdings } from "../../api/index.ts";
 import { get_url_path } from "../../helpers.ts";
 import { _ } from "../../i18n.ts";
 import { get_url_filters } from "../../stores/filters.ts";
@@ -17,56 +17,6 @@ const to_report_type = (s: string | null): HoldingsReportType =>
     ? s
     : "all";
 
-const QUERIES = {
-  all: `
-SELECT
-  account,
-  units(sum(position)) as units,
-  cost_number as cost,
-  first(getprice(currency, cost_currency)) as price,
-  cost(sum(position)) as book_value,
-  value(sum(position)) as market_value,
-  safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct,
-  cost_date as acquisition_date
-WHERE account_sortkey(account) ~ "^[01]"
-GROUP BY account, cost_date, currency, cost_currency, cost_number, account_sortkey(account)
-ORDER BY account_sortkey(account), currency, cost_date
-`.trim(),
-  by_account: `
-SELECT
-  account,
-  units(sum(position)) as units,
-  cost(sum(position)) as book_value,
-  value(sum(position)) as market_value,
-  safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct
-WHERE account_sortkey(account) ~ "^[01]"
-GROUP BY account, cost_currency, account_sortkey(account), currency
-ORDER BY account_sortkey(account), currency
-`.trim(),
-  by_currency: `
-SELECT
-  units(sum(position)) as units,
-  safediv(number(only(first(cost_currency), cost(sum(position)))), number(only(first(currency), units(sum(position))))) as average_cost,
-  first(getprice(currency, cost_currency)) as price,
-  cost(sum(position)) as book_value,
-  value(sum(position)) as market_value,
-  safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct
-WHERE account_sortkey(account) ~ "^[01]"
-GROUP BY currency, cost_currency
-ORDER BY currency, cost_currency
-`.trim(),
-  by_cost_currency: `
-SELECT
-  units(sum(position)) as units,
-  cost(sum(position)) as book_value,
-  value(sum(position)) as market_value,
-  safediv((abs(sum(number(value(position)))) - abs(sum(number(cost(position))))), sum(number(cost(position)))) * 100 as unrealized_profit_pct
-WHERE account_sortkey(account) ~ "^[01]"
-GROUP BY cost_currency
-ORDER BY cost_currency
-`.trim(),
-};
-
 export interface HoldingsReportProps {
   aggregation_key: HoldingsReportType;
   query_string: string;
@@ -79,9 +29,8 @@ export const holdings = new Route<HoldingsReportProps>(
   async (url) => {
     const [, key = ""] = get_url_path(url).unwrap().split("/");
     const aggregation_key = to_report_type(key);
-    const query_string = QUERIES[aggregation_key];
-    const query_result_table = await get_query({
-      query_string,
+    const { query_string, query_result_table } = await get_holdings({
+      aggregation_key,
       ...get_url_filters(url),
     });
     if (query_result_table.t !== "table") {

src/fava/core/holdings.py

@@ -0,0 +1,92 @@
+"""Holdings report queries.
+
+Beanquery's ``value()`` and ``getprice()`` use the latest price when no date
+is given. Holdings queries run against ``entries_with_all_prices``, which
+adds prices from outside the time filter back in, so an undated call would
+value lots at future prices. Pin both functions to the filter end date
+(the same date account reports use).
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:  # pragma: no cover
+    import datetime
+
+_UNREALIZED_PROFIT_PCT = (
+    "safediv((abs(sum(number(value(position{date}))))"
+    " - abs(sum(number(cost(position))))),"
+    " sum(number(cost(position)))) * 100 as unrealized_profit_pct"
+)
+_AVERAGE_COST = (
+    "safediv(number(only(first(cost_currency), cost(sum(position)))),"
+    " number(only(first(currency), units(sum(position))))) as average_cost"
+)
+
+HOLDINGS_QUERIES = {
+    "all": f"""
+SELECT
+  account,
+  units(sum(position)) as units,
+  cost_number as cost,
+  first(getprice(currency, cost_currency{{date}})) as price,
+  cost(sum(position)) as book_value,
+  value(sum(position){{date}}) as market_value,
+  {_UNREALIZED_PROFIT_PCT},
+  cost_date as acquisition_date
+WHERE account_sortkey(account) ~ "^[01]"
+GROUP BY account, cost_date, currency, cost_currency, cost_number,
+  account_sortkey(account)
+ORDER BY account_sortkey(account), currency, cost_date
+""".strip(),
+    "by_account": f"""
+SELECT
+  account,
+  units(sum(position)) as units,
+  cost(sum(position)) as book_value,
+  value(sum(position){{date}}) as market_value,
+  {_UNREALIZED_PROFIT_PCT}
+WHERE account_sortkey(account) ~ "^[01]"
+GROUP BY account, cost_currency, account_sortkey(account), currency
+ORDER BY account_sortkey(account), currency
+""".strip(),
+    "by_currency": f"""
+SELECT
+  units(sum(position)) as units,
+  {_AVERAGE_COST},
+  first(getprice(currency, cost_currency{{date}})) as price,
+  cost(sum(position)) as book_value,
+  value(sum(position){{date}}) as market_value,
+  {_UNREALIZED_PROFIT_PCT}
+WHERE account_sortkey(account) ~ "^[01]"
+GROUP BY currency, cost_currency
+ORDER BY currency, cost_currency
+""".strip(),
+    "by_cost_currency": f"""
+SELECT
+  units(sum(position)) as units,
+  cost(sum(position)) as book_value,
+  value(sum(position){{date}}) as market_value,
+  {_UNREALIZED_PROFIT_PCT}
+WHERE account_sortkey(account) ~ "^[01]"
+GROUP BY cost_currency
+ORDER BY cost_currency
+""".strip(),
+}
+
+
+def holdings_query(
+    aggregation_key: str, end_date: datetime.date | None
+) -> str:
+    """Return the holdings BQL, pinning prices to ``end_date`` when set.
+
+    Args:
+        aggregation_key: One of the keys in ``HOLDINGS_QUERIES``.
+        end_date: Inclusive date to value at, or ``None`` for the latest price.
+
+    Returns:
+        The BQL string for the holdings report.
+    """
+    date_arg = f", {end_date.isoformat()}" if end_date is not None else ""
+    return HOLDINGS_QUERIES[aggregation_key].format(date=date_arg)

src/fava/json_api.py

@@ -41,6 +41,8 @@
 from fava.core.file import get_entry_slice
 from fava.core.filters import FilterError
 from fava.core.group_entries import group_entries_by_type
+from fava.core.holdings import HOLDINGS_QUERIES
+from fava.core.holdings import holdings_query
 from fava.core.ingest import filepath_in_primary_imports_folder
 from fava.core.misc import align
 from fava.helpers import FavaAPIError
@@ -169,6 +171,15 @@ def __init__(self, filename: str) -> None:
         super().__init__(f"Not a file: '{filename}'")
 
 
+class UnknownHoldingsAggregationKeyError(ValidationError):
+    """Unknown holdings aggregation key."""
+
+    def __init__(self, aggregation_key: str) -> None:
+        super().__init__(
+            f"unknown holdings aggregation key: `{aggregation_key}`"
+        )
+
+
 @json_api.errorhandler(FavaAPIError)
 def _(error: FavaAPIError) -> Response:
     log.error("Encountered FavaAPIError.", exc_info=error)
@@ -339,6 +350,27 @@ def get_query(query_string: str) -> QueryResultTable | QueryResultText:
     )
 
 
+class HoldingsReport(Struct, frozen=True):
+    """Data for the holdings report."""
+
+    query_string: str
+    query_result_table: QueryResultTable | QueryResultText
+
+
+@api_endpoint
+def get_holdings(aggregation_key: str = "all") -> HoldingsReport:
+    """Get the holdings report, valued at the time-filter end date."""
+    if aggregation_key not in HOLDINGS_QUERIES:
+        raise UnknownHoldingsAggregationKeyError(aggregation_key)
+    query_string = holdings_query(aggregation_key, g.filtered.end_date)
+    return HoldingsReport(
+        query_string,
+        g.ledger.query_shell.execute_query_serialised(
+            g.filtered.entries_with_all_prices, query_string
+        ),
+    )
+
+
 @api_endpoint
 def get_extract(filename: str, importer: str) -> Sequence[object]:
     """Extract entries using the ingest framework."""

tests/test_core_holdings.py

@@ -0,0 +1,91 @@
+from __future__ import annotations
+
+from datetime import date
+from decimal import Decimal
+from textwrap import dedent
+from typing import TYPE_CHECKING
+
+import pytest
+
+from fava.core import FavaLedger
+from fava.core.holdings import HOLDINGS_QUERIES
+from fava.core.holdings import holdings_query
+from fava.core.query import QueryResultTable
+
+if TYPE_CHECKING:  # pragma: no cover
+    from pathlib import Path
+
+
+@pytest.mark.parametrize("aggregation_key", list(HOLDINGS_QUERIES))
+def test_holdings_query_pins_price_functions(aggregation_key: str) -> None:
+    query = holdings_query(aggregation_key, date(2017, 8, 31))
+    assert "value(sum(position), 2017-08-31)" in query
+    assert "value(position, 2017-08-31)" in query
+    if "getprice" in HOLDINGS_QUERIES[aggregation_key]:
+        assert "getprice(currency, cost_currency, 2017-08-31)" in query
+
+
+@pytest.mark.parametrize("aggregation_key", list(HOLDINGS_QUERIES))
+def test_holdings_query_omits_date_without_filter(
+    aggregation_key: str,
+) -> None:
+    query = holdings_query(aggregation_key, None)
+    assert "value(sum(position), " not in query
+    assert "value(position, " not in query
+    assert "getprice(currency, cost_currency, " not in query
+    assert "value(sum(position))" in query
+    assert "value(position)" in query
+
+
+def _account_market_value(
+    table: QueryResultTable, account: str
+) -> dict[str, Decimal]:
+    names = [col.name for col in table.types]
+    acc_i = names.index("account")
+    val_i = names.index("market_value")
+    row = next(r for r in table.rows if r[acc_i] == account)
+    value = row[val_i]
+    assert isinstance(value, dict)
+    return value
+
+
+def test_holdings_values_at_filter_end_date(tmp_path: Path) -> None:
+    """Undated value() uses later prices; holdings must pin the filter end."""
+    ledger_path = tmp_path / "prices.beancount"
+    ledger_path.write_text(
+        dedent("""\
+            option "title" "Holdings Prices"
+            option "operating_currency" "USD"
+
+            2020-01-01 open Assets:Broker
+            2020-01-01 open Assets:Cash USD
+            2020-01-01 commodity STOCK
+
+            2020-06-01 * "Buy"
+              Assets:Broker  10 STOCK {10.00 USD}
+              Assets:Cash   -100.00 USD
+
+            2020-06-30 price STOCK 12.00 USD
+            2020-09-30 price STOCK 20.00 USD
+            """)
+    )
+    ledger = FavaLedger(str(ledger_path))
+    filtered = ledger.get_filtered(time="2020-06")
+    assert filtered.end_date == date(2020, 6, 30)
+
+    undated = ledger.query_shell.execute_query_serialised(
+        filtered.entries_with_all_prices,
+        holdings_query("by_account", None),
+    )
+    dated = ledger.query_shell.execute_query_serialised(
+        filtered.entries_with_all_prices,
+        holdings_query("by_account", filtered.end_date),
+    )
+    assert isinstance(undated, QueryResultTable)
+    assert isinstance(dated, QueryResultTable)
+    assert _account_market_value(undated, "Assets:Broker") == {
+        "USD": Decimal("200.00")
+    }
+    assert _account_market_value(dated, "Assets:Broker") == {
+        "USD": Decimal("120.00")
+    }