chore(backlog): TODO.md orchestration round 3 — API list-shape convention completion + planned-payment fixes

alembic/versions/0050_add_accounts_entity.py

@@ -31,16 +31,30 @@
 checks. Distinct from `recipient_bank_accounts` (counterparty IBANs, ADR-015/ADR-087).
 
 Blast radius: one new table + two nullable columns + indexes (no rewrite of existing columns),
-plus a bounded backfill (one INSERT over distinct strings, two UPDATEs keyed by the new index).
+plus a bounded backfill (one INSERT over distinct strings, UPDATEs to link rows).
 No existing data is destroyed. Downgrade drops the columns/indexes, the table, and the enum
 types; the backfilled `account_id` values disappear with the columns (the `bank_account` string
 is untouched, so re-applying re-derives them).
 
+COST CONTROL (large installs): on a big `transactions` table the naive shape of step 3 — a
+single full-table UPDATE plus two eager index builds, all inside one migration transaction —
+is the most expensive step of the whole chain (full heap rewrite + full WAL + write-blocking
+index builds). To keep it bounded, the `transactions` half of the backfill and the two
+`transactions` indexes run inside an `autocommit_block()`:
+  * the backfill UPDATE is batched over id ranges, each batch committing on its own, so locks
+    stay short, WAL is spread out, and vacuum can reclaim dead tuples between batches;
+  * the indexes are built AFTER the backfill (no index maintenance during the rewrite) and
+    with CREATE INDEX CONCURRENTLY (no write-blocking lock);
+  * every statement stays idempotent (`WHERE account_id IS NULL`, drop-invalid-then-create),
+    so a kill mid-migration resumes cleanly on the next boot — the same resume contract
+    `transaction_per_migration` (alembic/env.py) already establishes per-migration.
+
 NOTE: migrations are not auto-run — this is AUTHORED NOT YET APPLIED; the user applies it.
 """
 
 from typing import Sequence, Union
 
+import sqlalchemy as sa
 from alembic import op
 
 
@@ -49,6 +63,45 @@
 branch_labels: Union[str, Sequence[str], None] = None
 depends_on: Union[str, Sequence[str], None] = None
 
+# Rows per committed batch of the transactions backfill UPDATE (id-range keyed, so each
+# batch is a cheap PK range scan regardless of how sparse the id space is).
+BACKFILL_BATCH_SIZE = 50_000
+
+
+def _create_index_concurrently(name: str, create_sql: str) -> None:
+    """CREATE INDEX CONCURRENTLY with the failure caveat handled.
+
+    Must be called inside an autocommit_block(): CONCURRENTLY cannot run in a
+    transaction. A concurrent build that is interrupted leaves an INVALID index
+    behind, and `CREATE INDEX IF NOT EXISTS` would silently keep it — so instead:
+    a valid existing index is kept (idempotent re-run, no wasteful rebuild), an
+    invalid leftover is dropped, and only then is the index (re)built.
+    """
+    valid = (
+        op.get_bind()
+        .execute(
+            sa.text(
+                """
+                SELECT i.indisvalid
+                  FROM pg_index i
+                  JOIN pg_class c ON c.oid = i.indexrelid
+                  JOIN pg_namespace n ON n.oid = c.relnamespace
+                 WHERE c.relname = :name
+                   AND n.nspname = current_schema()
+                """
+            ),
+            {"name": name},
+        )
+        .scalar()
+    )
+    if valid:
+        return
+    if valid is not None:  # exists but INVALID (interrupted previous concurrent build)
+        op.execute(f"DROP INDEX IF EXISTS {name}")
+    # One statement per execute: a multi-statement string would run as a single
+    # implicit transaction and CONCURRENTLY would refuse.
+    op.execute(create_sql)
+
 
 def upgrade() -> None:
     # ── Enum types for the categorical flags (idempotent, matching the 0001 idiom) ──
@@ -109,7 +162,10 @@ def upgrade() -> None:
         """
     )
 
-    # ── account_id FKs (ON DELETE RESTRICT) + indexes ──
+    # ── account_id FKs (ON DELETE RESTRICT) + planned_transactions index ──
+    # The two transactions indexes are NOT built here: they are built CONCURRENTLY at the
+    # end of upgrade(), after the backfill, inside the autocommit block (see docstring).
+    # planned_transactions is small, so its index stays in the transactional section.
     op.execute(
         """
         ALTER TABLE transactions
@@ -119,12 +175,6 @@ def upgrade() -> None:
             ADD COLUMN IF NOT EXISTS account_id INTEGER
             REFERENCES accounts(id) ON DELETE RESTRICT;
 
-        CREATE INDEX IF NOT EXISTS idx_transactions_account_id
-            ON transactions (account_id);
-        -- Mirrors idx_transactions_bank_date_active for the running-balance window
-        -- (PARTITION BY account_id ORDER BY date) and the balance-history LATERAL probe.
-        CREATE INDEX IF NOT EXISTS idx_transactions_account_date_active
-            ON transactions (account_id, date DESC) WHERE is_active = true;
         CREATE INDEX IF NOT EXISTS idx_planned_transactions_account_id
             ON planned_transactions (account_id);
         """
@@ -189,16 +239,10 @@ def upgrade() -> None:
         """
     )
 
-    # Link existing rows to their account by exact trimmed-name match.
+    # Link existing planned rows to their account by exact trimmed-name match
+    # (small table — single statement, stays in the migration transaction).
     op.execute(
         """
-        UPDATE transactions t
-           SET account_id = a.id
-          FROM accounts a
-         WHERE t.account_id IS NULL
-           AND t.bank_account IS NOT NULL
-           AND a.name = btrim(t.bank_account);
-
         UPDATE planned_transactions p
            SET account_id = a.id
           FROM accounts a
@@ -208,6 +252,57 @@ def upgrade() -> None:
         """
     )
 
+    # ── transactions backfill + indexes, outside the migration transaction ──
+    # autocommit_block() commits everything above (safe: every statement above is
+    # idempotent), runs the block in autocommit, then resumes a transaction for the
+    # alembic_version stamp. Semantics of the UPDATE are identical to the original
+    # single statement (same WHERE, same join) — it is merely partitioned by id range,
+    # and each batch commits on its own. The `account_id IS NULL` guard makes an
+    # interrupted backfill resume where it stopped on the next boot.
+    with op.get_context().autocommit_block():
+        bind = op.get_bind()
+        bounds = bind.execute(
+            sa.text("SELECT min(id) AS lo, max(id) AS hi FROM transactions")
+        ).one()
+        if bounds.lo is not None:
+            batch_lo = bounds.lo
+            while batch_lo <= bounds.hi:
+                batch_hi = batch_lo + BACKFILL_BATCH_SIZE - 1
+                bind.execute(
+                    sa.text(
+                        """
+                        UPDATE transactions t
+                           SET account_id = a.id
+                          FROM accounts a
+                         WHERE t.id BETWEEN :lo AND :hi
+                           AND t.account_id IS NULL
+                           AND t.bank_account IS NOT NULL
+                           AND a.name = btrim(t.bank_account)
+                        """
+                    ),
+                    {"lo": batch_lo, "hi": batch_hi},
+                )
+                batch_lo = batch_hi + 1
+
+        # Built AFTER the backfill so the heap rewrite above does not have to
+        # maintain them row-by-row, and CONCURRENTLY so writers are never blocked.
+        _create_index_concurrently(
+            "idx_transactions_account_id",
+            """
+            CREATE INDEX CONCURRENTLY idx_transactions_account_id
+                ON transactions (account_id)
+            """,
+        )
+        # Mirrors idx_transactions_bank_date_active for the running-balance window
+        # (PARTITION BY account_id ORDER BY date) and the balance-history LATERAL probe.
+        _create_index_concurrently(
+            "idx_transactions_account_date_active",
+            """
+            CREATE INDEX CONCURRENTLY idx_transactions_account_date_active
+                ON transactions (account_id, date DESC) WHERE is_active = true
+            """,
+        )
+
 
 def downgrade() -> None:
     op.execute(

apps/frontend/src/components/onboarding/__tests__/OnboardingWizard.test.tsx

@@ -16,11 +16,11 @@ function stubAdapters() {
     server.use(
         http.get(`${API_BASE}/api/info/supported-adapters`, () =>
             ok({
-                adapters: [
+                items: [
                     { key: "kbc", name: "KBC", adapter_class: "KbcAdapter" },
                     { key: "ing", name: "ING", adapter_class: "IngAdapter" },
                 ],
-                total_count: 2,
+                total: 2,
             }),
         ),
     );

apps/frontend/src/components/planned/PlannedPaymentForm.tsx

@@ -153,12 +153,10 @@ export default function PlannedPaymentForm({ open, onOpenChange, onSubmit, initi
     // If loan is enabled, clear recurrence inputs before submitting - loans drive their own schedule
     const payload: Record<string, unknown> = {
       name: name.trim(),
-      // Loans: POST overwrites this from the generated schedule outright, and
-      // PATCH keeps a defined client value as-is (never re-negates it), so
-      // there is no double negation on either path. Note PATCH keeping the
-      // client value means editing a loan's principal/rate/term can leave a
-      // stale amount vs. the regenerated schedule — a pre-existing backend
-      // behavior, tracked separately in TODO.md.
+      // Loans: the server owns the amount — POST and PATCH both re-derive it
+      // as -|regular_payment_amount| whenever the repayment schedule is
+      // (re)generated, ignoring this client value, so there is no double
+      // negation and no stale amount on either path.
       amount: signedAmount,
       currency,
       due_date: dueDateStr,

apps/frontend/src/components/planned/RecurringDetectionPanel.tsx

@@ -147,7 +147,15 @@ export function RecurringDetectionPanel({ onCreatePlanned }: Props) {
                 planned_date: pattern.predictedNext,
                 recipient_id: pattern.recipientId,
                 memo: t('recurring.autoDetectedMemo', { name: pattern.recipientName }),
-                amount: pattern.latestAmount * -1,
+                // Detected amounts are .abs()'d server-side; the pattern's
+                // `direction` carries the dominant sign of the source
+                // transactions. Planned sign convention: money out negative,
+                // money in positive — hardcoding the expense sign here turned a
+                // detected salary into a negative planned payment that
+                // plannedMatchService could never auto-match (sign mismatch).
+                amount: pattern.direction === "income"
+                    ? Math.abs(pattern.latestAmount)
+                    : -Math.abs(pattern.latestAmount),
                 currency: pattern.currency,
                 category_id: pattern.categoryId ?? undefined,
                 bank_account: pattern.bankAccount ?? undefined,

apps/frontend/src/components/planned/__tests__/RecurringDetectionPanel.test.tsx

@@ -0,0 +1,104 @@
+// @vitest-environment jsdom
+import { beforeEach, describe, expect, it } from "vitest";
+import { screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { http } from "msw";
+import { renderWithApp } from "@/test/renderWithApp";
+import { server } from "@/test/msw/server";
+import { ok } from "@/test/msw/handlers";
+import { RecurringDetectionPanel } from "@/components/planned/RecurringDetectionPanel";
+
+const API_BASE = "http://localhost:3002";
+
+/**
+ * Sign convention pin (planned-payments): money OUT is negative, money IN is
+ * positive. The detection service .abs()'s every amount server-side and carries
+ * the flow sign only in `direction` — the panel must recombine them, or a
+ * detected income (salary, rent received) becomes a negative planned payment
+ * that plannedMatchService can never auto-match against the real positive
+ * transaction.
+ */
+function patternFixture(direction: "income" | "expense") {
+    return {
+        recipientId: direction === "income" ? 7 : 8,
+        recipientName: direction === "income" ? "Employer NV" : "Landlord SA",
+        direction,
+        detectedPattern: "monthly",
+        intervalDays: 30,
+        consistency: 95,
+        occurrences: 6,
+        averageAmount: 2500, // .abs()'d server-side — always positive on the wire
+        latestAmount: 2500,
+        currency: "EUR",
+        categoryId: null,
+        categoryName: null,
+        bankAccount: "BE12",
+        firstSeen: "2025-01-28",
+        lastSeen: "2025-06-28",
+        predictedNext: "2025-07-28",
+        amountChanges: [],
+        isAlreadyPlanned: false,
+        confidence: 90,
+    };
+}
+
+function servePatternsAndCapturePost(direction: "income" | "expense") {
+    const captured: { body: Record<string, unknown> | null } = { body: null };
+    server.use(
+        http.get(`${API_BASE}/api/info/recurring-patterns`, () =>
+            ok({ patterns: [patternFixture(direction)], total: 1 }),
+        ),
+        http.post(`${API_BASE}/api/planned-transactions`, async ({ request }) => {
+            captured.body = (await request.json()) as Record<string, unknown>;
+            return ok({ id: 99 });
+        }),
+    );
+    return captured;
+}
+
+async function clickTrack(user: ReturnType<typeof userEvent.setup>) {
+    const trackBtn = await screen.findByRole("button", { name: /track/i });
+    await user.click(trackBtn);
+}
+
+describe("RecurringDetectionPanel — detected sign carried into the planned payment", () => {
+    beforeEach(() => {
+        window.localStorage.clear();
+    });
+
+    it("creates a POSITIVE planned amount for a detected income pattern", async () => {
+        const user = userEvent.setup();
+        const captured = servePatternsAndCapturePost("income");
+
+        renderWithApp(<RecurringDetectionPanel />);
+
+        expect(await screen.findByText("Employer NV")).toBeInTheDocument();
+        await clickTrack(user);
+
+        await waitFor(() => expect(captured.body).not.toBeNull());
+        expect(captured.body).toMatchObject({
+            amount: 2500, // income → positive, auto-matchable against the credit
+            recipient_id: 7,
+            currency: "EUR",
+            planned_date: "2025-07-28",
+            is_recurring: true,
+            recurrence_pattern: "monthly",
+        });
+    });
+
+    it("creates a NEGATIVE planned amount for a detected expense pattern", async () => {
+        const user = userEvent.setup();
+        const captured = servePatternsAndCapturePost("expense");
+
+        renderWithApp(<RecurringDetectionPanel />);
+
+        expect(await screen.findByText("Landlord SA")).toBeInTheDocument();
+        await clickTrack(user);
+
+        await waitFor(() => expect(captured.body).not.toBeNull());
+        expect(captured.body).toMatchObject({
+            amount: -2500, // expense → negative, auto-matchable against the debit
+            recipient_id: 8,
+        });
+    });
+});

apps/frontend/src/components/portfolio/AddToWatchlistDialog.tsx

@@ -98,8 +98,8 @@ export function AddToWatchlistDialog({ open, onOpenChange, prefill }: AddToWatch
     queryFn: async () => {
       if (!selectedAsset?.symbol) return null;
       try {
-        const data = await getMarketQuotes(selectedAsset.symbol, { detail: "basic" });
-        return data.quotes?.[0] ?? null;
+        const quotes = await getMarketQuotes(selectedAsset.symbol, { detail: "basic" });
+        return quotes[0] ?? null;
       } catch {
         return null;
       }

apps/frontend/src/components/portfolio/PortfolioNewsFeed.tsx

@@ -29,7 +29,7 @@ export function PortfolioNewsFeed({ symbols }: PortfolioNewsFeedProps) {
     enabled: isOnline,
   });
 
-  const articles = data?.articles ?? [];
+  const articles = data ?? [];
 
   return (
     <Card className="glass-regular h-full flex flex-col">

apps/frontend/src/components/portfolio/PortfolioTicker.tsx

@@ -123,7 +123,7 @@ export function PortfolioTicker({ items }: PortfolioTickerProps) {
 
   const entries = useMemo<TickerEntry[]>(() => {
     const bySymbol = new Map<string, TickerQuote>();
-    for (const q of data?.quotes ?? []) {
+    for (const q of data ?? []) {
       if (q.symbol) bySymbol.set(q.symbol.toUpperCase(), q);
     }
     if (bySymbol.size === 0) return [];

apps/frontend/src/components/portfolio/WatchlistChartDialog.tsx

@@ -74,7 +74,7 @@ export function WatchlistChartDialog({ item, open, onOpenChange }: WatchlistChar
     queryFn: async () => {
       if (!item?.symbol) return null;
       try {
-        const { quotes } = await apiClient.getMarketQuotes(item.symbol, { detail: "basic" });
+        const quotes = await apiClient.getMarketQuotes(item.symbol, { detail: "basic" });
         return quotes[0] ?? null;
       } catch {
         return null;