674b44d047
- agentic chat loop (max 6 round-trips, budget re-checked between iterations) over 10 read-only, permission-delegated tools in src/services/ai-tools.ts - persist assistant tool trace in ai_chat_messages.content_json (+ migration) - consulted-tools chips in OdinThread; header now shows month spend only (budget cap hidden from the UI, server-side 402 guard unchanged) - seed: ai.use permission; Settings exposes the ai permission module - docs: REVIEW consolidation; deployment-guide.md superseded by docs/release.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
125 lines
6.2 KiB
Markdown
125 lines
6.2 KiB
Markdown
# Deferred HIGH issues — performance & UX sprint
|
|
|
|
**Created:** 2026-06-03
|
|
**Source audit:** Parallel 5-agent production risk audit (2026-06-03)
|
|
**Context:** All 8 CRITICAL and 3 of 8 HIGH issues were fixed in the same session.
|
|
The 5 items below were intentionally deferred — they are not data-integrity or
|
|
auth risks, but they are real production pain that should be addressed in a
|
|
dedicated performance / UX sprint before the user base notices.
|
|
|
|
---
|
|
|
|
## #9 — N+1 queries in warehouse list/detail/report
|
|
|
|
**Files:**
|
|
|
|
- `src/services/warehouse.service.ts:184-199` (getBelowMinimumItems)
|
|
- `src/services/warehouse.service.ts:629-646` (somewhere in items list)
|
|
- `src/services/warehouse.service.ts:681-683`
|
|
- `src/services/warehouse.service.ts:2182-2203` (report)
|
|
|
|
**Pattern:** `items.map(async (i) => { const stock = await getStock(i.id); const available = await getAvailable(i.id); const value = await getValue(i.id); })` — fires N+1 round-trips per item. For a list of 50 items, that's 150 sequential queries.
|
|
|
|
**Production impact:** Slow page loads (1-3s on 50 items, scales linearly). Users already complain about the items page; this is the root cause for the warehouse report being the slowest page in the app.
|
|
|
|
**Fix sketch:**
|
|
|
|
- Replace the `for` loop with a single grouped query: `prisma.sklad_batches.groupBy({ by: ['item_id'], _sum: { quantity: true }, where: { item_id: { in: itemIds } } })` for stocks, similar aggregates for reservations.
|
|
- Map the grouped results back to items in JS. One query instead of N.
|
|
- For the report view, do the same — group all batches and reservations by item_id in a single round-trip, then join with items in memory.
|
|
- Estimated effort: M (4-6 hours including testing).
|
|
|
|
---
|
|
|
|
## #10 — Missing FK indexes on hot warehouse tables
|
|
|
|
**File:** `prisma/schema.prisma` (no `@@index` directives on the new warehouse tables)
|
|
|
|
**Missing indexes (verify against `prisma/schema.prisma` and add to a new migration):**
|
|
|
|
- `sklad_receipt_lines.item_id` — every `confirmReceipt` reads/writes this
|
|
- `sklad_issue_lines.batch_id` — every `confirmIssue` reads/writes this
|
|
- `sklad_issue_lines.item_id`
|
|
- `sklad_reservations.project_id` — reservation list per project
|
|
- `sklad_reservations.item_id`
|
|
- `sklad_item_locations.item_id`
|
|
- `sklad_item_locations.location_id`
|
|
- `sklad_batches.receipt_line_id` (if nullable FK)
|
|
- `sklad_inventory_lines.item_id`
|
|
- `sklad_inventory_lines.session_id`
|
|
|
|
**Production impact:** MySQL currently does table scans for these joins. With even 10k batch rows, the receipts/issues/reservations pages will start to crawl. On production data (likely already 50k+ batches), this is a real performance cliff.
|
|
|
|
**Fix sketch:**
|
|
|
|
- Add `@@index([item_id])`, `@@index([batch_id])` etc. to each model in `prisma/schema.prisma`.
|
|
- Run `npx prisma migrate dev --name warehouse_fk_indexes` (after asking user to stop dev server per CLAUDE.md).
|
|
- Run `npx prisma generate`.
|
|
- Verify with `EXPLAIN` on the slow queries after deploy.
|
|
- Estimated effort: S (1-2 hours).
|
|
|
|
**CLAUDE.md note:** Before running `prisma migrate dev`, the user must stop the dev server.
|
|
|
|
---
|
|
|
|
## #13 — Pagination does not auto-adjust after delete
|
|
|
|
**Files:** All list pages (Invoices, Offers, Projects, Orders, WarehouseXxx)
|
|
|
|
**Pattern:** User is on page 5 (e.g. 10 items per page, 47 total, pages 1-5). They delete the last item on page 5. Total becomes 46, but the page param is still 5. The list endpoint returns 0 results, the user sees an empty table, and the "next" button is disabled but there's no automatic jump back to page 4.
|
|
|
|
**Production impact:** Confusing UX. Users have to manually click "previous" or reset filters. Doesn't cause data loss but is a small papercut that erodes trust.
|
|
|
|
**Fix sketch:**
|
|
|
|
- In each list page's `useQuery` error/empty handler, detect `pagination.total === 0 && page > 1` and call `setPage(page - 1)`.
|
|
- Or more cleanly: have the API include `pagination.total_pages` and the frontend clamp `page` to `min(page, total_pages)` after each refetch.
|
|
- Estimated effort: S (2-3 hours; touches ~10-12 pages).
|
|
|
|
---
|
|
|
|
## #14 — Offer lock lifecycle has multiple silent failures
|
|
|
|
**File:** `src/admin/pages/OfferDetail.tsx:472-516`
|
|
|
|
**Pattern:** Lock acquire, heartbeat (interval), and unlock all use `.catch(() => {})`. If any of them silently fails, the user thinks they have the lock but actually don't — or worse, the lock is held by a tab that's already closed because the unload handler also swallowed the error.
|
|
|
|
**Production impact:** Two users editing the same offer can both see "You have the lock" and clobber each other's changes. Data loss in a real, low-probability but high-impact scenario.
|
|
|
|
**Fix sketch:**
|
|
|
|
- Replace `.catch(() => {})` with `.catch((err) => console.error("Offer lock error:", err))` at minimum.
|
|
- Better: surface lock acquisition failures via a toast (`alert.error("Nepodařilo se získat zámek, stránka bude pouze pro čtení.")`).
|
|
- For the unlock on unmount: best-effort `navigator.sendBeacon` or a synchronous fallback so the lock is released even if the page is closing.
|
|
- Estimated effort: S (1-2 hours).
|
|
|
|
---
|
|
|
|
## #11 (audit item, not a real bug) — TOTP replay counter rewind
|
|
|
|
**File investigated:** `src/utils/totp.ts:5-30`, `src/routes/admin/auth.ts:165-184`
|
|
|
|
**Status:** False positive. The audit suggested `verifyResult.counter` could be lower than the actual step used. After reading the implementation:
|
|
|
|
```ts
|
|
const delta = totp.validate({ token: code, window: 1 });
|
|
// ...
|
|
const counterDelta = Math.min(delta, 0); // -1 or 0, never +1
|
|
const counter = currentCounter + counterDelta;
|
|
```
|
|
|
|
`Math.min(delta, 0)` clamps to ≤ 0, so `counter` is always the **current** step or **one step in the past** — never a future step. The `counter <= lastCounter` check in the route is correct and complete. No fix needed.
|
|
|
|
If anyone revisits this in the future, this analysis is the basis for closing the ticket.
|
|
|
|
---
|
|
|
|
## Suggested order for the next sprint
|
|
|
|
1. **#10 (indexes)** — cheap, high impact, addresses a growing data cliff. Do first.
|
|
2. **#9 (N+1)** — bigger effort but the same root cause family. Tackle after indexes.
|
|
3. **#14 (offer lock)** — small but prevents data loss; fits in a single PR.
|
|
4. **#13 (pagination)** — UX polish; do last, batch with other UX cleanup.
|
|
|
|
Estimated total: 1-2 days of focused work.
|