chore(backlog): TODO.md orchestration round 4 — category resolution, DB harness, ratchet

.github/workflows/ci.yml

@@ -228,8 +228,9 @@ jobs:
   # Two steps, one program: the base config is loose (strict/noImplicitAny off)
   # and covers all of src/; the ratchet re-runs it with noImplicitAny: true and
   # fails only on the paths listed in scripts/checkjs-ratchet.js (the data-layer
-  # row contracts + the repositories annotated against them). See that script's
-  # header for why this is not a second directory-scoped tsconfig.
+  # row contracts, every repository, and the service subdirectories annotated
+  # against them). See that script's header for why this is not a second
+  # directory-scoped tsconfig, and its RATCHETED list for what is still out.
   # ─────────────────────────────────────────────────────────────────────────────
   typecheck-backend:
     name: Type Check (Backend)

alembic/versions/0085_mv_monthly_summary_alias_category.py

@@ -0,0 +1,59 @@
+"""Rebuild mv_monthly_summary with the 3-level effective-category resolution.
+
+Revision ID: 0085_mv_monthly_summary_alias_category
+Revises: 0084_mv_category_totals_alias_category
+Create Date: 2026-07-28
+
+`mv_monthly_summary` (the getMonthlyFinancialSummary fast path) resolved the
+effective category over TWO levels only — `COALESCE(t.category_id,
+r.default_category_id)` — while the transactions surfaces resolve THREE
+(`…, pr.default_category_id`): a row recorded under an alias recipient whose
+PRIMARY carries the default category was categorised in the transactions list
+but grouped under the UNCATEGORISED (`category_id_key = -1`) row of the monthly
+view. The monthly live path in `infoRepositoryMonthly.getMonthlyFinancialSummary`
+is fixed to the 3-level pattern in the same release; this migration drops the MV
+so the fast path is rebuilt from the corrected definition and the two paths
+cannot disagree on the category grain.
+
+The view's only current reader sums across categories, so the monthly totals
+themselves are unchanged — this aligns the stored category grain (and the
+`category_id_key` unique-index column) with every other money surface rather
+than fixing a wrong total.
+
+`CREATE MATERIALIZED VIEW IF NOT EXISTS` never redefines an existing view, so
+without this drop already-migrated installs would keep the old 2-level SQL
+forever (REFRESH re-runs the *stored* definition). Same drop-and-let-boot-
+recreate precedent as migrations 0045 and 0084: materializedViewService.
+createMaterializedViews runs right after migrations on the same boot
+(main.js), recreates the view with the new definition, and the startup /
+post-mutation refresh populates it. Until then `mvAvailable()` sees it
+unpopulated and getMonthlyFinancialSummary serves the (corrected) live query —
+no window of wrong answers.
+
+Downgrade drops the view again so the older code recreates its prior 2-level
+definition on next boot. Derived data only; nothing to preserve.
+"""
+
+from typing import Sequence, Union
+
+from alembic import op
+
+
+revision: str = "0085_mv_monthly_summary_alias_category"
+down_revision: Union[str, Sequence[str], None] = "0084_mv_category_totals_alias_category"
+branch_labels: Union[str, Sequence[str], None] = None
+depends_on: Union[str, Sequence[str], None] = None
+
+
+def upgrade() -> None:
+    # destructive-ok: derived data only, rebuilt from transactions by
+    # materializedViewService.createMaterializedViews on the same boot that applies this
+    # migration (precedent: 0045, 0084) — the recreation with the 3-level effective-category
+    # definition is the point of the drop. Its unique index drops with it via CASCADE.
+    op.execute("DROP MATERIALIZED VIEW IF EXISTS mv_monthly_summary CASCADE;")
+
+
+def downgrade() -> None:
+    # destructive-ok: same rebuild-at-boot contract in reverse — older application code
+    # recreates the previous 2-level definition via createMaterializedViews on next start.
+    op.execute("DROP MATERIALIZED VIEW IF EXISTS mv_monthly_summary CASCADE;")

apps/frontend/src/lib/api/imports.ts

@@ -50,7 +50,7 @@ const IMPORT_STREAM_SCHEMAS: Record<string, z.ZodType> = {
 export function importCSV(
     file: File,
     bankName: string,
-): Promise<{ batch_id: string; imported: number; duplicates: number; total_processed: number; message: string }> {
+): Promise<{ batch_id: number; imported: number; duplicates: number; total_processed: number; message: string }> {
     const queryParams = new URLSearchParams();
     queryParams.append('bank_name', bankName);
     return postMultipartImport('/api/import/csv', file, queryParams);
@@ -155,7 +155,7 @@ export function importCSVCustom(
     separator: string = ',',
     encoding: string = 'utf-8',
     skipRows: number = 0,
-): Promise<{ batch_id: string; imported: number; duplicates: number; total_processed: number; message: string }> {
+): Promise<{ batch_id: number; imported: number; duplicates: number; total_processed: number; message: string }> {
     const queryParams = new URLSearchParams();
     queryParams.append('bank_name', bankName);
     queryParams.append('date_format', dateFormat);

apps/frontend/src/types/generated.ts

@@ -8498,7 +8498,7 @@ export interface operations {
                 content: {
                     "application/json": components["schemas"]["Envelope"] & {
                         data?: {
-                            batch_id?: string;
+                            batch_id?: number;
                             total?: number;
                             imported?: number;
                             duplicates?: number;
@@ -8515,7 +8515,7 @@ export interface operations {
                 content: {
                     "application/json": components["schemas"]["Envelope"] & {
                         data?: {
-                            batch_id?: string;
+                            batch_id?: number;
                             requires_review?: boolean;
                             match_source_counts?: {
                                 [key: string]: number;

apps/node-backend/package.json

@@ -24,6 +24,7 @@
     "archiver": "^8.0.0",
     "eslint": "^10.7.0",
     "globals": "^17.7.0",
+    "supertest": "^7.2.2",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10",
     "yauzl": "^3.4.0"

apps/node-backend/scripts/checkjs-ratchet.js

@@ -45,19 +45,117 @@ const CONFIG_PATH = path.join(ROOT, 'tsconfig.check.strict.json');
  * Seeded with the core data-layer row shapes (`src/types/rows.js`), then grown
  * file-by-file until every repository was annotated. The whole data layer is
  * now held as directory prefixes, so a NEW file under either directory is
- * ratcheted from birth. Per the original plan, the next targets are further
- * directories (services/, routes/, …) — grow them file-by-file, then collapse
- * to the directory prefix once complete.
+ * ratcheted from birth.
+ *
+ * `src/services/` is being taken the same way, one whole subdirectory at a
+ * time: a subdirectory is annotated to zero errors and only then added here as
+ * a prefix, so new files under it are ratcheted from birth too. Held so far as
+ * prefixes: the two import pipelines, currency/FX, prices, tax, and info.
+ *
+ * The top level of `src/services/` (all 51 `*.js` files directly in that
+ * directory, not its subdirectories) is now fully held — but still as 51
+ * individual file paths, not a `src/services/` prefix: `isRatcheted`'s prefix
+ * match is per-string-prefix, not per-directory-level, so a `src/services/`
+ * entry would ALSO match every still-dirty subdirectory below (`aiChat/` and
+ * most of `calculations/`/`research/`/`reports/`/`portfolio/`) and gate them
+ * sight-unseen — instant CI failure on this script's next run. Collapsing the
+ * 51 entries to a directory prefix is only safe once those subdirectories are
+ * themselves fully clean (at which point `src/services/` alone, without the
+ * now-redundant subdirectory prefixes, would cover the whole tree).
+ *
+ * `calculations/`, `research/`, `reports/`, and `portfolio/` are each
+ * individually-clean-file-only so far (not yet whole subdirectories) — most
+ * of those directories are still implicit-any-dirty. `aiChat/` is untouched.
+ * Beyond services: `routes/` and `lib/` are untouched.
  *
  * @type {string[]}
  */
 const RATCHETED = [
   'src/types/',
   'src/repositories/',
+  'src/services/currency/',
+  'src/services/importPipeline/',
+  'src/services/info/',
+  'src/services/portfolioImportPipeline/',
+  'src/services/prices/',
+  'src/services/tax/',
+
+  // calculations/, research/, reports/, portfolio/ — individually-clean files
+  // only; the directories themselves are still mostly implicit-any-dirty.
+  'src/services/calculations/aggregation/_envelope.js',
+  'src/services/calculations/aggregation/_statisticsCache.js',
+  'src/services/calculations/aggregation/averageVsCurrent.js',
+  'src/services/calculations/aggregation/bankBalances.js',
+  'src/services/calculations/aggregation/category.js',
+  'src/services/calculations/forecast/methods/ensemble.js',
+  'src/services/calculations/forecast/methods/simpleAverage.js',
+  'src/services/calculations/normalization.js',
+  'src/services/portfolio/portfolioIncomeService.js',
+  'src/services/reports/dataFetcherPortfolio.js',
+  'src/services/reports/sectionCatalog.js',
+  'src/services/research/adapters/schemas.js',
+  'src/services/research/providerRegistry.js',
+
+  // Top level of src/services/ — all 51 files, alphabetical.
+  'src/services/accountMergeService.js',
+  'src/services/accountService.js',
+  'src/services/aggregationRefresh.js',
+  'src/services/aiChatService.js',
+  'src/services/attachmentCleanup.js',
+  'src/services/attachmentRecordService.js',
+  'src/services/attachmentService.js',
+  'src/services/bankAdapters.js',
+  'src/services/belgianInflationService.js',
+  'src/services/bulkSelection.js',
+  'src/services/cashForecastInsightService.js',
+  'src/services/categoryOutlierService.js',
+  'src/services/categoryService.js',
+  'src/services/crossWorkspaceAnalytics.js',
+  'src/services/crossWorkspaceDataService.js',
+  'src/services/customParserConfigService.js',
+  'src/services/dataImportService.js',
+  'src/services/dbEditor.js',
+  'src/services/deduplication.js',
+  'src/services/importBatchService.js',
+  'src/services/infoService.js',
+  'src/services/insightsDigestService.js',
+  'src/services/marketLookupService.js',
+  'src/services/materializedViewService.js',
+  'src/services/openingBalanceService.js',
+  'src/services/plannedExecutionService.js',
+  'src/services/plannedMatchService.js',
+  'src/services/plannedTransactionService.js',
+  'src/services/portfolioImportBatchService.js',
+  'src/services/portfolioPerformanceSnapshotService.js',
+  'src/services/priceProviderService.js',
+  'src/services/providerHealthService.js',
+  'src/services/quoteBackfillService.js',
+  'src/services/recipientBankAccountService.js',
+  'src/services/recipientClusterService.js',
+  'src/services/recipientMergeService.js',
+  'src/services/recipientPatternService.js',
+  'src/services/recipientService.js',
+  'src/services/reconcileService.js',
+  'src/services/recurringDetectionService.js',
+  'src/services/routeManifest.js',
+  'src/services/savedChartsService.js',
+  'src/services/settingsService.js',
+  'src/services/splitService.js',
+  'src/services/subscriptionCreepService.js',
+  'src/services/tagService.js',
+  'src/services/transactionBulkService.js',
+  'src/services/transactionExport.js',
+  'src/services/transactionService.js',
+  'src/services/transferReconciliationService.js',
+  'src/services/watchlistService.js',
 ];
 
-/** Directory the "ready to ratchet" hint scans for already-clean files. */
-const HINT_SCOPE = 'src/repositories/';
+/**
+ * Directory the "ready to ratchet" hint scans for already-clean files. Points
+ * at the frontier: `src/repositories/` is complete, so the hint now surfaces
+ * service files that are already implicit-any-clean and could be listed.
+ */
+const HINT_SCOPE = 'src/services/';
 
 /**
  * @param {string} absolutePath

apps/node-backend/src/middleware/errorHandler.js

@@ -8,6 +8,10 @@
  *
  * Untyped errors fall through to a 500 with the production-safe message used by
  * the previous inline handler in main.js. Production mode hides raw messages.
+ *
+ * One exception: an untyped error that carries its own 4xx status (body-parser's
+ * http-errors, raised before any route runs) keeps that status. See THE RULE
+ * above `forwardable4xx` for exactly when, and when its message is echoed.
  */
 
 import { logger } from '../config/logger.js';
@@ -74,6 +78,106 @@ export class RateLimitedError extends AppError {
   }
 }
 
+/* ── Non-AppError errors that carry their own HTTP status ─────────────────
+ *
+ * Some errors reaching this handler are not ours and never will be: body-parser
+ * (mounted by `express.json()` in main.js:130) rejects a request BEFORE any
+ * route runs and raises an `http-errors` instance carrying the correct status —
+ * 400 for truncated JSON, 413 for a body over the 1 MB cap. Collapsing those to
+ * 500 reports a client typo as a server fault, and in production the 5xx
+ * sanitizer then hides the one thing the client needed to know ("request entity
+ * too large").
+ *
+ * THE RULE (two independent decisions — status, then message):
+ *
+ *  1. STATUS is forwarded when a non-AppError carries a numeric `status` or
+ *     `statusCode` in the 400-499 range. 5xx and nonsense values (NaN, 0, 700,
+ *     strings) are ignored and still take the sanitized 500 path, so an
+ *     internal failure can never downgrade itself into a client error.
+ *
+ *  2. MESSAGE is echoed verbatim ONLY when the error also carries a `type` from
+ *     `TRUSTED_ERROR_TYPES` below — body-parser's fixed, non-sensitive strings.
+ *     Every other forwarded 4xx gets the generic reason phrase for its status.
+ *     Reason: `.status`/`.statusCode` is a convention any library may adopt,
+ *     and its message is not vetted. In this codebase the only other non-AppError
+ *     with a status is `OllamaError` (integrations/ollama/client.js:25), which
+ *     stores the UPSTREAM provider's HTTP status and a message naming our
+ *     internal call ("Ollama POST /api/chat failed with 404"); it is normally
+ *     wrapped into an AppError (aiChatService.js:325), but if one ever escapes,
+ *     this rule keeps the wording out of the response. Same for the plain
+ *     `err.statusCode = 400` throws in services/calculations/loanSchedule.js:70,102
+ *     — they get a truthful 400 without this handler vouching for their text.
+ *
+ * `details` is still AppError-only: nothing here fabricates one.
+ */
+
+/** body-parser `type` values whose message is a fixed library string, safe to echo. */
+const TRUSTED_ERROR_TYPES = new Set([
+  'entity.parse.failed', // 400 — malformed/truncated JSON body
+  'entity.too.large', // 413 — body over the express.json({ limit }) cap
+  'parameters.too.many', // 413 — urlencoded parameter-count cap
+  'request.aborted', // 400 — client hung up mid-body
+  'request.size.invalid', // 400 — Content-Length disagreed with the body read
+  'charset.unsupported', // 415
+  'encoding.unsupported', // 415
+  'entity.verify.failed', // 403 — an express.json({ verify }) hook rejected it
+]);
+
+/** Generic reason phrases for a forwarded 4xx whose own message is not trusted. */
+const GENERIC_4XX_MESSAGES = {
+  400: 'Bad Request',
+  401: 'Unauthorized',
+  403: 'Forbidden',
+  404: 'Not Found',
+  405: 'Method Not Allowed',
+  409: 'Conflict',
+  413: 'Payload Too Large',
+  415: 'Unsupported Media Type',
+  422: 'Unprocessable Content',
+  429: 'Too Many Requests',
+};
+
+/**
+ * Map a forwarded 4xx status onto the stable client-visible code vocabulary
+ * (ADR-026). The list is deliberately the existing one — no new codes are
+ * minted for these, since every UI already branches on VALIDATION_ERROR for a
+ * "your request was wrong" outcome.
+ *
+ * @param {number} status
+ * @returns {string}
+ */
+function codeForForwardedStatus(status) {
+  switch (status) {
+    case 401: return ApiErrorCode.UNAUTHORIZED;
+    case 403: return ApiErrorCode.FORBIDDEN;
+    case 404: return ApiErrorCode.NOT_FOUND;
+    case 409: return ApiErrorCode.CONFLICT;
+    case 429: return ApiErrorCode.RATE_LIMITED;
+    // 400 / 405 / 413 / 415 / 422 / any other 4xx: the request itself was
+    // rejected — VALIDATION_ERROR is the code clients already handle for that.
+    default: return ApiErrorCode.VALIDATION_ERROR;
+  }
+}
+
+/**
+ * Decide whether a non-AppError may keep its own status, per THE RULE above.
+ *
+ * @param {any} err
+ * @returns {{ status: number, code: string, message: string }|null} null when the
+ *   error must take the ordinary 500 path.
+ */
+function forwardable4xx(err) {
+  const raw = typeof err.status === 'number' ? err.status : err.statusCode;
+  if (typeof raw !== 'number' || !Number.isInteger(raw) || raw < 400 || raw > 499) return null;
+
+  const trusted = typeof err.type === 'string' && TRUSTED_ERROR_TYPES.has(err.type);
+  return {
+    status: raw,
+    code: codeForForwardedStatus(raw),
+    message: trusted ? err.message : (GENERIC_4XX_MESSAGES[raw] || 'Request rejected'),
+  };
+}
+
 /**
  * Factory: returns Express error-handling middleware bound to the provided
  * `isProduction` predicate. Injecting the predicate keeps this module free of
@@ -96,12 +200,18 @@ export function createErrorHandler(isProduction) {
     }
 
     const isApp = err instanceof AppError;
-    const status = isApp ? err.status : 500;
-    const code = isApp ? err.code : ApiErrorCode.INTERNAL_SERVER_ERROR;
+    // Errors raised before any route ran (body-parser) carry a correct 4xx of
+    // their own — see THE RULE above for when it is honoured.
+    const forwarded = isApp ? null : forwardable4xx(err);
+    const status = isApp ? err.status : (forwarded ? forwarded.status : 500);
+    const code = isApp ? err.code : (forwarded ? forwarded.code : ApiErrorCode.INTERNAL_SERVER_ERROR);
 
     // Typed 4xx errors are expected business outcomes — log at warn, not error.
     const logFn = status >= 500 ? logger.error : logger.warn;
-    logFn.call(logger, isApp ? 'Handled application error' : 'Unhandled exception', {
+    const logLabel = isApp
+      ? 'Handled application error'
+      : (forwarded ? 'Rejected request' : 'Unhandled exception');
+    logFn.call(logger, logLabel, {
       error: err.message,
       code,
       status,
@@ -112,7 +222,13 @@ export function createErrorHandler(isProduction) {
     });
 
     let message;
-    if (status < 500) {
+    if (forwarded) {
+      // Not ours: body-parser's fixed strings pass through, everything else
+      // gets the generic reason phrase. Unlike the 5xx branch this is NOT
+      // environment-dependent — the whole point is that a client hitting the
+      // body-size cap in production can still read why.
+      message = forwarded.message;
+    } else if (status < 500) {
       // 4xx messages are authored by us — safe to expose.
       message = err.message;
     } else if (isProduction()) {