diff --git a/app/features/analytics/routes.py b/app/features/analytics/routes.py index 762d5204..161b655a 100644 --- a/app/features/analytics/routes.py +++ b/app/features/analytics/routes.py @@ -6,11 +6,12 @@ from datetime import date -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Query from sqlalchemy.ext.asyncio import AsyncSession from app.core.config import get_settings from app.core.database import get_db +from app.core.exceptions import BadRequestError from app.core.logging import get_logger from app.features.analytics.schemas import ( DrilldownDimension, @@ -40,23 +41,23 @@ def validate_date_range(start_date: date, end_date: date) -> None: end_date: End of analysis period. Raises: - HTTPException: If date range is invalid. + BadRequestError: If date range is invalid. Surfaces as an RFC 7807 + ``application/problem+json`` 400 via the registered handler — a + raw ``HTTPException`` would bypass the problem-details envelope. """ settings = get_settings() if end_date < start_date: - raise HTTPException( - status_code=400, - detail=f"end_date ({end_date}) must be >= start_date ({start_date})", + raise BadRequestError( + message=f"end_date ({end_date}) must be >= start_date ({start_date})", ) days_diff = (end_date - start_date).days max_days = settings.analytics_max_date_range_days if days_diff > max_days: - raise HTTPException( - status_code=400, - detail=f"Date range ({days_diff} days) exceeds maximum allowed ({max_days} days)", + raise BadRequestError( + message=f"Date range ({days_diff} days) exceeds maximum allowed ({max_days} days)", ) @@ -108,10 +109,12 @@ async def get_kpis( ), store_id: int | None = Query( None, + ge=1, description="Filter by store ID. Use GET /dimensions/stores to find valid IDs.", ), product_id: int | None = Query( None, + ge=1, description="Filter by product ID. Use GET /dimensions/products to find valid IDs.", ), category: str | None = Query( @@ -134,7 +137,7 @@ async def get_kpis( Aggregated KPI metrics. Raises: - HTTPException: If date range is invalid. + BadRequestError: If date range is invalid (RFC 7807 400). """ # Validate date range before processing validate_date_range(start_date, end_date) @@ -206,10 +209,12 @@ async def get_drilldowns( ), store_id: int | None = Query( None, + ge=1, description="Filter by store ID. Use GET /dimensions/stores to find valid IDs.", ), product_id: int | None = Query( None, + ge=1, description="Filter by product ID. Use GET /dimensions/products to find valid IDs.", ), max_items: int = Query( @@ -235,7 +240,7 @@ async def get_drilldowns( Drilldown analysis with ranked items. Raises: - HTTPException: If date range is invalid. + BadRequestError: If date range is invalid (RFC 7807 400). """ # Validate date range before processing validate_date_range(start_date, end_date) @@ -301,10 +306,12 @@ async def get_timeseries( ), store_id: int | None = Query( None, + ge=1, description="Filter by store ID. Use GET /dimensions/stores to find valid IDs.", ), product_id: int | None = Query( None, + ge=1, description="Filter by product ID. Use GET /dimensions/products to find valid IDs.", ), category: str | None = Query( @@ -328,7 +335,7 @@ async def get_timeseries( Time series response with points in ascending period order. Raises: - HTTPException: If date range is invalid. + BadRequestError: If date range is invalid (RFC 7807 400). """ # Validate date range before processing validate_date_range(start_date, end_date) @@ -380,10 +387,12 @@ async def get_timeseries( async def get_inventory_status( store_id: int | None = Query( None, + ge=1, description="Filter by store ID. Use GET /dimensions/stores to find valid IDs.", ), product_id: int | None = Query( None, + ge=1, description="Filter by product ID. Use GET /dimensions/products to find valid IDs.", ), db: AsyncSession = Depends(get_db), diff --git a/app/features/analytics/tests/conftest.py b/app/features/analytics/tests/conftest.py index b101fabc..a6aa09cd 100644 --- a/app/features/analytics/tests/conftest.py +++ b/app/features/analytics/tests/conftest.py @@ -7,7 +7,7 @@ import pytest from httpx import ASGITransport, AsyncClient -from sqlalchemy import delete +from sqlalchemy import delete, select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from app.core.config import get_settings @@ -122,11 +122,25 @@ async def db_session() -> AsyncGenerator[AsyncSession, None]: try: yield session finally: - # Clean up test data (delete in FK-safe order). InventorySnapshotDaily - # FK-references store/product/calendar, so it must be cleared before - # the Store/Product/Calendar deletes below. - await session.execute(delete(InventorySnapshotDaily)) - await session.execute(delete(SalesDaily)) + # Clean up test data (delete in FK-safe order). Scope the fact-table + # deletes to TEST-prefixed stores/products so a shared dev or + # integration dataset is never wiped. InventorySnapshotDaily / + # SalesDaily FK-reference store/product/calendar, so they must be + # cleared before the Store/Product/Calendar deletes below. + test_store_ids = select(Store.id).where(Store.code.like("TEST-%")) + test_product_ids = select(Product.id).where(Product.sku.like("TEST-%")) + await session.execute( + delete(InventorySnapshotDaily).where( + InventorySnapshotDaily.store_id.in_(test_store_ids) + | InventorySnapshotDaily.product_id.in_(test_product_ids) + ) + ) + await session.execute( + delete(SalesDaily).where( + SalesDaily.store_id.in_(test_store_ids) + | SalesDaily.product_id.in_(test_product_ids) + ) + ) await session.execute(delete(Product).where(Product.sku.like("TEST-%"))) await session.execute(delete(Store).where(Store.code.like("TEST-%"))) await session.execute( @@ -154,7 +168,9 @@ async def override_get_db() -> AsyncGenerator[AsyncSession, None]: ) as ac: yield ac - app.dependency_overrides.clear() + # Remove only this fixture's override — clear() would also drop overrides + # installed by other fixtures sharing the app instance. + app.dependency_overrides.pop(get_db, None) @pytest.fixture diff --git a/app/features/config/tests/test_service.py b/app/features/config/tests/test_service.py index e97009c1..1bcdfd99 100644 --- a/app/features/config/tests/test_service.py +++ b/app/features/config/tests/test_service.py @@ -101,19 +101,34 @@ async def test_get_effective_config_masks_secrets(self): async def test_get_effective_config_maps_agent_limits(self): """The agent session-limit fields are sourced from the Settings singleton.""" settings = get_settings() - settings.agent_max_tool_calls = 7 - settings.agent_timeout_seconds = 99 - settings.agent_retry_attempts = 2 - settings.agent_session_ttl_minutes = 45 - settings.agent_require_approval = ["create_alias"] - - config = await service.get_effective_config(_mock_db()) - - assert config.agent_max_tool_calls == 7 - assert config.agent_timeout_seconds == 99 - assert config.agent_retry_attempts == 2 - assert config.agent_session_ttl_minutes == 45 - assert config.agent_require_approval == ["create_alias"] + # get_settings() returns a cached singleton — snapshot every field this + # test mutates and restore it in a finally block so the mutation never + # leaks into another test. + fields = ( + "agent_max_tool_calls", + "agent_timeout_seconds", + "agent_retry_attempts", + "agent_session_ttl_minutes", + "agent_require_approval", + ) + original = {field: getattr(settings, field) for field in fields} + try: + settings.agent_max_tool_calls = 7 + settings.agent_timeout_seconds = 99 + settings.agent_retry_attempts = 2 + settings.agent_session_ttl_minutes = 45 + settings.agent_require_approval = ["create_alias"] + + config = await service.get_effective_config(_mock_db()) + + assert config.agent_max_tool_calls == 7 + assert config.agent_timeout_seconds == 99 + assert config.agent_retry_attempts == 2 + assert config.agent_session_ttl_minutes == 45 + assert config.agent_require_approval == ["create_alias"] + finally: + for field, value in original.items(): + setattr(settings, field, value) # ============================================================================= diff --git a/app/features/dimensions/service.py b/app/features/dimensions/service.py index 54854aba..a77af056 100644 --- a/app/features/dimensions/service.py +++ b/app/features/dimensions/service.py @@ -108,9 +108,11 @@ async def list_stores( else: order_by = Store.code.asc() - # Apply pagination + # Apply pagination. Append the unique `code` as a tie-breaker so rows + # with equal sort values keep a stable order across pages (offset + # pagination over a non-unique sort key is otherwise non-deterministic). offset = (page - 1) * page_size - stmt = stmt.order_by(order_by).offset(offset).limit(page_size) + stmt = stmt.order_by(order_by, Store.code.asc()).offset(offset).limit(page_size) # Execute query result = await db.execute(stmt) @@ -233,9 +235,11 @@ async def list_products( else: order_by = Product.sku.asc() - # Apply pagination + # Apply pagination. Append the unique `sku` as a tie-breaker so rows + # with equal sort values keep a stable order across pages (offset + # pagination over a non-unique sort key is otherwise non-deterministic). offset = (page - 1) * page_size - stmt = stmt.order_by(order_by).offset(offset).limit(page_size) + stmt = stmt.order_by(order_by, Product.sku.asc()).offset(offset).limit(page_size) # Execute query result = await db.execute(stmt) diff --git a/app/features/dimensions/tests/conftest.py b/app/features/dimensions/tests/conftest.py index 9befdffe..fb9b12a5 100644 --- a/app/features/dimensions/tests/conftest.py +++ b/app/features/dimensions/tests/conftest.py @@ -7,7 +7,7 @@ import pytest from httpx import ASGITransport, AsyncClient -from sqlalchemy import delete +from sqlalchemy import delete, select from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine from app.core.config import get_settings @@ -66,8 +66,17 @@ async def db_session() -> AsyncGenerator[AsyncSession, None]: try: yield session finally: - # Clean up test data (delete in FK-safe order). - await session.execute(delete(SalesDaily)) + # Clean up test data (delete in FK-safe order). Scope the SalesDaily + # delete to TEST-prefixed stores/products so a shared dev or + # integration dataset is never wiped. + test_store_ids = select(Store.id).where(Store.code.like("TEST-%")) + test_product_ids = select(Product.id).where(Product.sku.like("TEST-%")) + await session.execute( + delete(SalesDaily).where( + SalesDaily.store_id.in_(test_store_ids) + | SalesDaily.product_id.in_(test_product_ids) + ) + ) await session.execute(delete(Product).where(Product.sku.like("TEST-%"))) await session.execute(delete(Store).where(Store.code.like("TEST-%"))) await session.execute( @@ -95,7 +104,9 @@ async def override_get_db() -> AsyncGenerator[AsyncSession, None]: ) as ac: yield ac - app.dependency_overrides.clear() + # Remove only this fixture's override — clear() would also drop overrides + # installed by other fixtures sharing the app instance. + app.dependency_overrides.pop(get_db, None) @pytest.fixture diff --git a/app/features/jobs/service.py b/app/features/jobs/service.py index e559fb99..528bb20e 100644 --- a/app/features/jobs/service.py +++ b/app/features/jobs/service.py @@ -261,9 +261,16 @@ async def list_jobs( else: order_by = Job.created_at.desc() - # Apply pagination + # Apply pagination. Append created_at then the unique `job_id` as + # tie-breakers so rows with equal sort values keep a stable order + # across pages (offset pagination over a non-unique sort key is + # otherwise non-deterministic). offset = (page - 1) * page_size - stmt = stmt.order_by(order_by).offset(offset).limit(page_size) + stmt = ( + stmt.order_by(order_by, Job.created_at.desc(), Job.job_id.asc()) + .offset(offset) + .limit(page_size) + ) # Execute query result = await db.execute(stmt) diff --git a/app/features/jobs/tests/conftest.py b/app/features/jobs/tests/conftest.py index 41589643..2777253c 100644 --- a/app/features/jobs/tests/conftest.py +++ b/app/features/jobs/tests/conftest.py @@ -71,7 +71,9 @@ async def override_get_db() -> AsyncGenerator[AsyncSession, None]: ) as ac: yield ac - app.dependency_overrides.clear() + # Remove only this fixture's override — clear() would also drop overrides + # installed by other fixtures sharing the app instance. + app.dependency_overrides.pop(get_db, None) @pytest.fixture diff --git a/app/features/registry/service.py b/app/features/registry/service.py index ceb0b4bf..076910c1 100644 --- a/app/features/registry/service.py +++ b/app/features/registry/service.py @@ -322,9 +322,11 @@ async def list_runs( else: order_by = ModelRun.created_at.desc() - # Apply pagination + # Apply pagination. Append the unique `run_id` as a tie-breaker so rows + # with equal sort values keep a stable order across pages (offset + # pagination over a non-unique sort key is otherwise non-deterministic). offset = (page - 1) * page_size - stmt = stmt.order_by(order_by).offset(offset).limit(page_size) + stmt = stmt.order_by(order_by, ModelRun.run_id.asc()).offset(offset).limit(page_size) result = await db.execute(stmt) runs = result.scalars().all() diff --git a/frontend/src/components/charts/backtest-folds-chart.tsx b/frontend/src/components/charts/backtest-folds-chart.tsx index bc2e0ffa..98b7500d 100644 --- a/frontend/src/components/charts/backtest-folds-chart.tsx +++ b/frontend/src/components/charts/backtest-folds-chart.tsx @@ -67,7 +67,10 @@ export function BacktestFoldsChart({ {description && {description}} - + {/* Height is passed via inline style — a `h-[${height}px]` class is a + dynamic string Tailwind cannot statically discover, so the JIT + compiler drops it at build time. */} + diff --git a/frontend/src/components/charts/revenue-bar-chart.tsx b/frontend/src/components/charts/revenue-bar-chart.tsx index 58ff87b9..200c19f2 100644 --- a/frontend/src/components/charts/revenue-bar-chart.tsx +++ b/frontend/src/components/charts/revenue-bar-chart.tsx @@ -41,7 +41,10 @@ export function RevenueBarChart({ {description && {description}} - + {/* Height is passed via inline style — a `h-[${height}px]` class is a + dynamic string Tailwind cannot statically discover, so the JIT + compiler drops it at build time. */} + diff --git a/frontend/src/components/charts/time-series-chart.tsx b/frontend/src/components/charts/time-series-chart.tsx index b32bf641..bb999458 100644 --- a/frontend/src/components/charts/time-series-chart.tsx +++ b/frontend/src/components/charts/time-series-chart.tsx @@ -70,7 +70,10 @@ export function TimeSeriesChart({ {description && {description}} - + {/* Height is passed via inline style — a `h-[${height}px]` class is a + dynamic string Tailwind cannot statically discover, so the JIT + compiler drops it at build time. */} + ({ {isLoading ? ( - // Loading skeleton + // Loading skeleton — one cell per *visible* column so the + // skeleton width matches the rendered table after a + // column-visibility toggle. Array.from({ length: pagination.pageSize }).map((_, i) => ( - {columns.map((_, j) => ( - + {table.getVisibleLeafColumns().map((column) => ( + ))} @@ -113,6 +115,20 @@ export function DataTable({ key={row.id} data-state={row.getIsSelected() && 'selected'} onClick={onRowClick ? () => onRowClick(row.original) : undefined} + // A clickable row must be keyboard-operable: expose it as a + // button to assistive tech and fire on Enter / Space. + onKeyDown={ + onRowClick + ? (event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + onRowClick(row.original) + } + } + : undefined + } + tabIndex={onRowClick ? 0 : undefined} + role={onRowClick ? 'button' : undefined} className={cn(onRowClick && 'cursor-pointer')} > {row.getVisibleCells().map((cell) => ( @@ -128,7 +144,7 @@ export function DataTable({ ) : ( {emptyMessage} diff --git a/frontend/src/hooks/index.ts b/frontend/src/hooks/index.ts index 43fdc939..e49a0c50 100644 --- a/frontend/src/hooks/index.ts +++ b/frontend/src/hooks/index.ts @@ -3,6 +3,7 @@ export * from './use-products' export * from './use-kpis' export * from './use-drilldowns' export * from './use-timeseries' +export * from './use-inventory' export * from './use-lifecycle-curve' export * from './use-runs' export * from './use-jobs' diff --git a/frontend/src/lib/csv-export.test.ts b/frontend/src/lib/csv-export.test.ts index 4112527c..7678f3c8 100644 --- a/frontend/src/lib/csv-export.test.ts +++ b/frontend/src/lib/csv-export.test.ts @@ -53,4 +53,17 @@ describe('toCsv', () => { ] expect(toCsv([{ a: null, b: undefined }], nullableColumns)).toBe('A,B\r\n,') }) + + it('neutralizes CSV formula injection by prefixing a quote', () => { + interface AttackRow { + val: string + } + const attackColumns: CsvColumn[] = [{ key: 'val', header: 'Val' }] + expect(toCsv([{ val: '=SUM(A1:A2)' }], attackColumns)).toBe("Val\r\n'=SUM(A1:A2)") + expect(toCsv([{ val: '+1+1' }], attackColumns)).toBe("Val\r\n'+1+1") + expect(toCsv([{ val: '-2+3' }], attackColumns)).toBe("Val\r\n'-2+3") + expect(toCsv([{ val: '@cmd' }], attackColumns)).toBe("Val\r\n'@cmd") + // A neutralized value that also needs RFC 4180 quoting is still wrapped. + expect(toCsv([{ val: '=1,2' }], attackColumns)).toBe('Val\r\n"\'=1,2"') + }) }) diff --git a/frontend/src/lib/csv-export.ts b/frontend/src/lib/csv-export.ts index 692cbdc7..3ceb8737 100644 --- a/frontend/src/lib/csv-export.ts +++ b/frontend/src/lib/csv-export.ts @@ -6,7 +6,13 @@ export interface CsvColumn { /** Quote a single CSV field per RFC 4180 (wrap + double internal quotes). */ function quoteField(value: unknown): string { - const str = value === null || value === undefined ? '' : String(value) + let str = value === null || value === undefined ? '' : String(value) + // CSV formula injection: a spreadsheet executes a cell whose value begins + // with =, +, -, @, or a control char (tab / CR). Prefix a single quote so + // the value is rendered as literal text instead of an evaluated formula. + if (/^[=+\-@\t\r]/.test(str)) { + str = `'${str}` + } if (/[",\r\n]/.test(str)) { return `"${str.replace(/"/g, '""')}"` } diff --git a/frontend/src/lib/demand-utils.test.ts b/frontend/src/lib/demand-utils.test.ts index e4294481..6f877553 100644 --- a/frontend/src/lib/demand-utils.test.ts +++ b/frontend/src/lib/demand-utils.test.ts @@ -122,6 +122,11 @@ describe('inventoryRequirement', () => { it('treats a null on-order as zero', () => { expect(inventoryRequirement(100, 30, null)).toBe(70) }) + + it('rounds a fractional shortfall up to avoid under-ordering', () => { + // shortfall = 10.4 - 5 - 0 = 5.4 → ceil → 6 (Math.round would yield 5) + expect(inventoryRequirement(10.4, 5, 0)).toBe(6) + }) }) describe('extractForecasts', () => { @@ -186,6 +191,20 @@ describe('joinDemandRows', () => { expect(row.inventoryRequirement).toBeNull() }) + it('keeps the latest inventory snapshot per grain by date', () => { + const job = makePredictJob('j1', { + store_id: 1, + product_id: 10, + forecasts: flatForecasts(7, 1), + }) + const older: InventoryStatusItem = { ...makeInventory(1, 10, 99, 0), date: '2026-01-01' } + const newer: InventoryStatusItem = { ...makeInventory(1, 10, 12, 3), date: '2026-02-01' } + // `newer` listed first — a naive Map-from-entries would keep `older`. + const [row] = joinDemandRows([job], products, [newer, older], 7) + expect(row.onHand).toBe(12) + expect(row.onOrder).toBe(3) + }) + it('falls back to a #id SKU when the product is unknown', () => { const job = makePredictJob('j1', { store_id: 1, diff --git a/frontend/src/lib/demand-utils.ts b/frontend/src/lib/demand-utils.ts index 8be1b9fc..fc475269 100644 --- a/frontend/src/lib/demand-utils.ts +++ b/frontend/src/lib/demand-utils.ts @@ -43,7 +43,9 @@ export function inventoryRequirement( onOrder: number | null, ): number | null { if (onHand === null) return null - return Math.max(0, Math.round(leadTimeDemand - onHand - (onOrder ?? 0))) + // Round up: a fractional shortfall still needs a whole extra unit ordered — + // Math.round would under-order when the fraction is below 0.5. + return Math.max(0, Math.ceil(leadTimeDemand - onHand - (onOrder ?? 0))) } /** @@ -86,9 +88,17 @@ export function joinDemandRows( leadTimeDays: number, ): DemandRow[] { const productById = new Map(products.map((product) => [product.id, product])) - const inventoryByGrain = new Map( - inventory.map((item) => [`${item.store_id}:${item.product_id}`, item]), - ) + // Keep the latest snapshot per grain. The API returns one row per grain, but + // a naive Map-from-entries would silently keep whichever row is last in the + // array — pick by `date` so order in the response can never matter. + const inventoryByGrain = new Map() + for (const item of inventory) { + const key = `${item.store_id}:${item.product_id}` + const existing = inventoryByGrain.get(key) + if (!existing || item.date > existing.date) { + inventoryByGrain.set(key, item) + } + } const rows: DemandRow[] = [] for (const job of predictJobs) { diff --git a/frontend/src/lib/url-params.test.ts b/frontend/src/lib/url-params.test.ts new file mode 100644 index 00000000..fc81d889 --- /dev/null +++ b/frontend/src/lib/url-params.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest' +import { parseEnumParam, parseIdParam, parsePageParam } from './url-params' + +describe('parsePageParam', () => { + it('returns the integer for a valid positive page', () => { + expect(parsePageParam('3')).toBe(3) + }) + + it('floors a fractional page above 1', () => { + expect(parsePageParam('2.9')).toBe(2) + }) + + it('falls back to 1 for null, non-numeric, zero, and negative input', () => { + expect(parsePageParam(null)).toBe(1) + expect(parsePageParam('')).toBe(1) + expect(parsePageParam('abc')).toBe(1) + expect(parsePageParam('0')).toBe(1) + expect(parsePageParam('-4')).toBe(1) + }) +}) + +describe('parseIdParam', () => { + it('returns a positive integer ID', () => { + expect(parseIdParam('42')).toBe(42) + }) + + it('returns undefined for null, empty, non-numeric, fractional, or non-positive input', () => { + expect(parseIdParam(null)).toBeUndefined() + expect(parseIdParam('')).toBeUndefined() + expect(parseIdParam('abc')).toBeUndefined() + expect(parseIdParam('1.5')).toBeUndefined() + expect(parseIdParam('0')).toBeUndefined() + expect(parseIdParam('-3')).toBeUndefined() + }) +}) + +describe('parseEnumParam', () => { + const allowed = ['asc', 'desc'] as const + + it('returns the value when it is a member of the allow-list', () => { + expect(parseEnumParam('desc', allowed)).toBe('desc') + }) + + it('returns undefined for null or an unknown value', () => { + expect(parseEnumParam(null, allowed)).toBeUndefined() + expect(parseEnumParam('sideways', allowed)).toBeUndefined() + }) +}) diff --git a/frontend/src/lib/url-params.ts b/frontend/src/lib/url-params.ts new file mode 100644 index 00000000..a9270ed0 --- /dev/null +++ b/frontend/src/lib/url-params.ts @@ -0,0 +1,48 @@ +/** + * Safe readers for URL query-string state. + * + * The explorer pages treat the query string as the single source of truth for + * filter / sort / page state, so a hand-edited, stale, or truncated URL can + * carry a NaN page, a negative page, or an unknown enum value. These helpers + * validate at the read boundary so a junk param degrades to a sane default + * instead of reaching a hook (and the API) unverified. + */ + +/** + * Parse a `page` query param into a positive integer (>= 1). + * + * `null`, non-numeric, zero, negative, and fractional inputs all fall back to + * `1`; a fractional input above 1 is floored (`"2.9"` -> `2`). + */ +export function parsePageParam(value: string | null): number { + const parsed = Number(value) + if (!Number.isFinite(parsed) || parsed < 1) return 1 + return Math.floor(parsed) +} + +/** + * Parse a positive-integer ID query param (`store_id`, `product_id`, ...). + * + * Returns `undefined` for `null`, empty, non-numeric, fractional, or + * non-positive input — never `NaN`. + */ +export function parseIdParam(value: string | null): number | undefined { + if (value === null || value === '') return undefined + const parsed = Number(value) + return Number.isInteger(parsed) && parsed >= 1 ? parsed : undefined +} + +/** + * Return `value` only when it is a member of `allowed`; otherwise `undefined`. + * + * Use for enum-typed query params (status, model_type, sort_by, dimension, ...) + * so an unknown value is dropped rather than blind-cast into a typed slot. + */ +export function parseEnumParam( + value: string | null, + allowed: readonly T[], +): T | undefined { + return value !== null && (allowed as readonly string[]).includes(value) + ? (value as T) + : undefined +} diff --git a/frontend/src/pages/explorer/job-detail.tsx b/frontend/src/pages/explorer/job-detail.tsx index e112b18a..77b15fed 100644 --- a/frontend/src/pages/explorer/job-detail.tsx +++ b/frontend/src/pages/explorer/job-detail.tsx @@ -21,6 +21,7 @@ import { AlertDialogTitle, AlertDialogTrigger, } from '@/components/ui/alert-dialog' +import { toast } from 'sonner' import { ROUTES } from '@/lib/constants' function fmtDate(value: string | null | undefined): string { @@ -74,10 +75,16 @@ export default function JobDetailPage() { const job = jobQuery.data async function handleCancel() { - await cancelJob.mutateAsync(jobId) - // useCancelJob invalidates ['jobs']; refresh this detail query explicitly - // so the page reflects the cancelled status immediately. - void queryClient.invalidateQueries({ queryKey: ['jobs', jobId] }) + // mutateAsync rejects on failure — catch it so a cancel error surfaces as + // a toast instead of an unhandled promise rejection. + try { + await cancelJob.mutateAsync(jobId) + // useCancelJob invalidates ['jobs']; refresh this detail query explicitly + // so the page reflects the cancelled status immediately. + void queryClient.invalidateQueries({ queryKey: ['jobs', jobId] }) + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to cancel job') + } } return ( diff --git a/frontend/src/pages/explorer/jobs.tsx b/frontend/src/pages/explorer/jobs.tsx index 380a732c..ee864dc5 100644 --- a/frontend/src/pages/explorer/jobs.tsx +++ b/frontend/src/pages/explorer/jobs.tsx @@ -21,7 +21,9 @@ import { AlertDialogTitle, AlertDialogTrigger, } from '@/components/ui/alert-dialog' +import { toast } from 'sonner' import { toCsv, downloadCsv, type CsvColumn } from '@/lib/csv-export' +import { parsePageParam } from '@/lib/url-params' import type { Job, JobStatus, JobType } from '@/types/api' import { DEFAULT_PAGE_SIZE } from '@/lib/constants' @@ -42,7 +44,9 @@ export default function JobsMonitorPage() { // so a pasted URL reproduces the exact view. const jobType = searchParams.get('job_type') ?? undefined const status = searchParams.get('status') ?? undefined - const page = Number(searchParams.get('page')) || 1 + // Clamp `page` to a positive integer — a hand-edited NaN/negative value + // would otherwise reach the API as-is. + const page = parsePageParam(searchParams.get('page')) const sortBy = searchParams.get('sort_by') ?? undefined const sortOrder: 'asc' | 'desc' = searchParams.get('sort_order') === 'desc' ? 'desc' : 'asc' @@ -64,7 +68,13 @@ export default function JobsMonitorPage() { const cancelJob = useCancelJob() const handleCancelJob = async (jobId: string) => { - await cancelJob.mutateAsync(jobId) + // mutateAsync rejects on failure — catch it so a cancel error surfaces as + // a toast instead of an unhandled promise rejection. + try { + await cancelJob.mutateAsync(jobId) + } catch (err) { + toast.error(err instanceof Error ? err.message : 'Failed to cancel job') + } } const columns: ColumnDef[] = [ diff --git a/frontend/src/pages/explorer/products.tsx b/frontend/src/pages/explorer/products.tsx index 893ee81e..ecc604c9 100644 --- a/frontend/src/pages/explorer/products.tsx +++ b/frontend/src/pages/explorer/products.tsx @@ -9,6 +9,7 @@ import { ErrorDisplay } from '@/components/common/error-display' import { Button } from '@/components/ui/button' import { formatCurrency } from '@/lib/api' import { toCsv, downloadCsv, type CsvColumn } from '@/lib/csv-export' +import { parsePageParam } from '@/lib/url-params' import type { Product } from '@/types/api' import { DEFAULT_PAGE_SIZE } from '@/lib/constants' @@ -62,7 +63,9 @@ export default function ProductsExplorerPage() { // URL query string is the single source of truth for filter/sort/page state. const search = searchParams.get('search') ?? '' const category = searchParams.get('category') ?? undefined - const page = Number(searchParams.get('page')) || 1 + // Clamp `page` to a positive integer — a hand-edited NaN/negative value + // would otherwise reach the API as-is. + const page = parsePageParam(searchParams.get('page')) const sortBy = searchParams.get('sort_by') ?? undefined const sortOrder: 'asc' | 'desc' = searchParams.get('sort_order') === 'desc' ? 'desc' : 'asc' diff --git a/frontend/src/pages/explorer/runs.tsx b/frontend/src/pages/explorer/runs.tsx index c6fb43fc..5fc1bf63 100644 --- a/frontend/src/pages/explorer/runs.tsx +++ b/frontend/src/pages/explorer/runs.tsx @@ -11,9 +11,16 @@ import { getStatusVariant } from '@/lib/status-utils' import { ErrorDisplay } from '@/components/common/error-display' import { Button } from '@/components/ui/button' import { toCsv, downloadCsv, type CsvColumn } from '@/lib/csv-export' +import { parseEnumParam, parsePageParam } from '@/lib/url-params' import type { ModelRun, RunStatus } from '@/types/api' import { DEFAULT_PAGE_SIZE, ROUTES } from '@/lib/constants' +// Allow-lists for the URL-driven filter/sort params. A hand-edited URL value +// outside these is dropped (treated as "no filter") rather than blind-cast. +const RUN_STATUSES: readonly RunStatus[] = ['pending', 'running', 'success', 'failed', 'archived'] +const MODEL_TYPES = ['naive', 'seasonal_naive', 'moving_average', 'lightgbm'] as const +const RUN_SORT_KEYS = ['created_at', 'model_type', 'status', 'store_id', 'product_id'] as const + const columns: ColumnDef[] = [ { accessorKey: 'run_id', @@ -89,11 +96,12 @@ export default function RunsExplorerPage() { const [searchParams, setSearchParams] = useSearchParams() // URL query string is the single source of truth for filter/sort/page state, - // so a pasted URL reproduces the exact view. - const modelType = searchParams.get('model_type') ?? undefined - const status = searchParams.get('status') ?? undefined - const page = Number(searchParams.get('page')) || 1 - const sortBy = searchParams.get('sort_by') ?? undefined + // so a pasted URL reproduces the exact view. Each param is validated so a + // junk value degrades gracefully instead of reaching the API. + const modelType = parseEnumParam(searchParams.get('model_type'), MODEL_TYPES) + const status = parseEnumParam(searchParams.get('status'), RUN_STATUSES) + const page = parsePageParam(searchParams.get('page')) + const sortBy = parseEnumParam(searchParams.get('sort_by'), RUN_SORT_KEYS) const sortOrder: 'asc' | 'desc' = searchParams.get('sort_order') === 'desc' ? 'desc' : 'asc' const pagination: PaginationState = { @@ -106,7 +114,7 @@ export default function RunsExplorerPage() { page, pageSize: pagination.pageSize, modelType, - status: status as RunStatus | undefined, + status, sortBy, sortOrder: sortBy ? sortOrder : undefined, }) diff --git a/frontend/src/pages/explorer/sales.tsx b/frontend/src/pages/explorer/sales.tsx index ba32036b..eff5ee6d 100644 --- a/frontend/src/pages/explorer/sales.tsx +++ b/frontend/src/pages/explorer/sales.tsx @@ -14,18 +14,28 @@ import { Badge } from '@/components/ui/badge' import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '@/components/ui/card' import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs' import { dateRangeToStrings, stringsToDateRange } from '@/lib/date-utils' +import { parseEnumParam, parseIdParam } from '@/lib/url-params' import { formatCurrency, formatNumber } from '@/lib/api' import type { DrilldownDimension } from '@/types/api' +/** The drilldown dimensions a shareable URL is allowed to select. */ +const DRILLDOWN_DIMENSIONS: readonly DrilldownDimension[] = [ + 'store', + 'product', + 'category', + 'region', + 'date', +] + export default function SalesExplorerPage() { const [searchParams, setSearchParams] = useSearchParams() // dimension + cross-filter state live in the URL so the view is shareable. - const dimension = (searchParams.get('dimension') as DrilldownDimension | null) ?? 'store' - const storeIdParam = searchParams.get('store_id') - const productIdParam = searchParams.get('product_id') - const storeId = storeIdParam ? Number(storeIdParam) : undefined - const productId = productIdParam ? Number(productIdParam) : undefined + // A hand-edited URL can carry an unknown dimension or a NaN id — validate + // both before they reach the drilldown/timeseries hooks. + const dimension = parseEnumParam(searchParams.get('dimension'), DRILLDOWN_DIMENSIONS) ?? 'store' + const storeId = parseIdParam(searchParams.get('store_id')) + const productId = parseIdParam(searchParams.get('product_id')) const startParam = searchParams.get('start_date') const endParam = searchParams.get('end_date') diff --git a/frontend/src/pages/explorer/stores.tsx b/frontend/src/pages/explorer/stores.tsx index 99914107..b5df3c39 100644 --- a/frontend/src/pages/explorer/stores.tsx +++ b/frontend/src/pages/explorer/stores.tsx @@ -8,9 +8,17 @@ import { DataTableColumnHeader } from '@/components/data-table/data-table-column import { ErrorDisplay } from '@/components/common/error-display' import { Button } from '@/components/ui/button' import { toCsv, downloadCsv, type CsvColumn } from '@/lib/csv-export' +import { parseEnumParam, parsePageParam } from '@/lib/url-params' import type { Store } from '@/types/api' import { DEFAULT_PAGE_SIZE } from '@/lib/constants' +// Allow-lists for the URL-driven filter/sort params — these mirror the filter +// dropdown options and the backend sort allow-list. A value outside them is +// dropped rather than blind-cast. +const REGIONS = ['North', 'South', 'East', 'West'] as const +const STORE_TYPES = ['Supermarket', 'Convenience', 'Hypermarket'] as const +const STORE_SORT_KEYS = ['code', 'name', 'region', 'city', 'store_type'] as const + const columns: ColumnDef[] = [ { accessorKey: 'id', @@ -59,12 +67,13 @@ export default function StoresExplorerPage() { const [searchParams, setSearchParams] = useSearchParams() // URL query string is the single source of truth for filter/sort/page state, - // so a pasted URL reproduces the exact view. + // so a pasted URL reproduces the exact view. Each param is validated so a + // junk value degrades gracefully instead of reaching the API. const search = searchParams.get('search') ?? '' - const region = searchParams.get('region') ?? undefined - const storeType = searchParams.get('store_type') ?? undefined - const page = Number(searchParams.get('page')) || 1 - const sortBy = searchParams.get('sort_by') ?? undefined + const region = parseEnumParam(searchParams.get('region'), REGIONS) + const storeType = parseEnumParam(searchParams.get('store_type'), STORE_TYPES) + const page = parsePageParam(searchParams.get('page')) + const sortBy = parseEnumParam(searchParams.get('sort_by'), STORE_SORT_KEYS) const sortOrder: 'asc' | 'desc' = searchParams.get('sort_order') === 'desc' ? 'desc' : 'asc' const pagination: PaginationState = { diff --git a/frontend/src/pages/guide.tsx b/frontend/src/pages/guide.tsx index 787a24e8..fe18ca6f 100644 --- a/frontend/src/pages/guide.tsx +++ b/frontend/src/pages/guide.tsx @@ -196,8 +196,12 @@ export default function GuidePage() { {tool} )) - ) : ( + ) : configLoading ? ( + ) : ( + + Unavailable — the configuration endpoint could not be reached. + )} diff --git a/frontend/src/pages/visualize/backtest.tsx b/frontend/src/pages/visualize/backtest.tsx index a2ce517f..3e36d763 100644 --- a/frontend/src/pages/visualize/backtest.tsx +++ b/frontend/src/pages/visualize/backtest.tsx @@ -85,7 +85,15 @@ export default function BacktestPage() { // Extract backtest result from job const backtestResult = job?.result as BacktestResult | undefined - const formReady = !!storeId && !!productId && !!dateRange?.from && !!dateRange?.to + // The number inputs can be cleared to 0; require a valid split count and + // test size so an invalid backtest config can never be submitted. + const formReady = + !!storeId && + !!productId && + !!dateRange?.from && + !!dateRange?.to && + nSplits >= 2 && + testSize >= 1 async function handleRunBacktest() { if (!storeId || !productId || !dateRange?.from || !dateRange?.to) return @@ -210,7 +218,8 @@ export default function BacktestPage() { {!formReady && ( - Pick a store, product and date window to enable. + Pick a store, product and date window, with at least 2 splits and a 1-day test + size, to enable. )} diff --git a/frontend/src/pages/visualize/demand.tsx b/frontend/src/pages/visualize/demand.tsx index 46f549d5..30bf7204 100644 --- a/frontend/src/pages/visualize/demand.tsx +++ b/frontend/src/pages/visualize/demand.tsx @@ -83,6 +83,17 @@ function SortHead({ onSort(columnKey)} + // Keyboard-operable sort header: focusable, fires on Enter / Space, and + // exposes the current sort direction to assistive tech. + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + onSort(columnKey) + } + }} + tabIndex={0} + role="button" + aria-sort={active ? (sortDir === 'asc' ? 'ascending' : 'descending') : 'none'} > {label} @@ -304,6 +315,15 @@ export default function DemandPlannerPage() { setSelectedJobId(row.jobId)} + // Keyboard-operable row: focusable and fires on Enter / Space. + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + setSelectedJobId(row.jobId) + } + }} + tabIndex={0} + role="button" className={cn( 'cursor-pointer', row.jobId === selectedJobId && 'bg-muted', diff --git a/frontend/src/pages/visualize/forecast.tsx b/frontend/src/pages/visualize/forecast.tsx index bda6f244..611864ec 100644 --- a/frontend/src/pages/visualize/forecast.tsx +++ b/frontend/src/pages/visualize/forecast.tsx @@ -48,8 +48,13 @@ export default function ForecastPage() { // A completed `predict` job stores result.forecasts (date + forecast, plus // optional lower/upper bounds for models that emit a prediction interval). - const forecastData = job?.result?.forecasts as ForecastPoint[] | undefined - const hasBounds = !!forecastData?.some( + // `job.result` is untyped JSONB — guard with Array.isArray before treating + // `forecasts` as an array so a malformed result can never throw on `.some()`. + const rawForecasts = job?.result?.forecasts + const forecastData: ForecastPoint[] = Array.isArray(rawForecasts) + ? (rawForecasts as ForecastPoint[]) + : [] + const hasBounds = forecastData.some( (point) => point.lower_bound != null && point.upper_bound != null, ) @@ -68,7 +73,7 @@ export default function ForecastPage() { } function handleExport() { - if (!forecastData || !job) return + if (forecastData.length === 0 || !job) return downloadCsv(`forecast-${job.job_id}.csv`, toCsv(forecastData, csvColumns)) } @@ -189,7 +194,7 @@ export default function ForecastPage() { {/* Forecast Chart */} - {forecastData && forecastData.length > 0 ? ( + {forecastData.length > 0 ? (