feat(app): scan-mode sync, confirmation-gated documents, single-pass product classification
Fixes reported from APK field testing: DO/Product scan mode was inconsistent between the camera drawer and documents screen (now one shared provider, with an orange/green color cue); unconfirmed scans leaked into history with placeholder data before the user tapped confirm (backend now gates GET /documents on a new `confirmed` column, flipped only by PUT); and Product Scan ran the GPU classifier twice, once at upload and again on review (now a single pass at upload, persisted and read directly by the editor). Also removes the unused "Hubungkan ke PO" field and fabricated PO/SO/DO placeholder values from the Product Scan flow, closes out the per-document-polling and save-recovery tasks (6.1/6.3), and splits several touched files to stay under the repo's 256-line guideline. Full detail in docs/iteration-log.md and backend/docs/iteration-log.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rafhan Mazaya FathurrahmanandClaude Sonnet 5 committed
1 parent
2febe0c886
commit
ada6488592
67 files changed
+5705
-1142
No files matched your search
@@ -44,12 +44,17 @@ workflow and have no task numbers; see `git log` for real dates/history.
|
||||
- **1.4** Enforced real 401 auth on `/api/v1/documents/*` (list, PUT-by-id, upload) — the actual production API surface, already fully supported by the Flutter client (real login + `Authorization: Bearer` on every request). Previously none of these three routes rejected a missing/invalid token; upload only optionally read it. Added the pre-existing `getAccountFromAuthHeader()` helper (`utils/auth.ts`) + a 401 guard to all three; `OPTIONS` (CORS preflight) untouched. The original task 1.3 (auth on the *classic* routes) was cancelled instead — those routes are dev-only web UI surface with no login flow, going away in production. Verified via `curl`: 401 with no token, success with a real token from `/api/v1/auth/login` — shipped 2026-07-08.
|
||||
- **1.5** Implemented per-store data scoping on `/api/v1/documents/*`. Added `kode_toko` column to `documents` table via `db/init.ts` migration. The upload route now binds `kode_toko` to documents upon creation. `GET /api/v1/documents` and `PUT /api/v1/documents/:id` enforce ownership checks (`kode_toko` matching) for `store` role accounts, while `admin` retains global access including legacy unassigned documents — shipped 2026-07-08.
|
||||
- **1.6** `GET /api/v1/health` Endpoint: Unauthenticated health probe verifying both PostgreSQL connectivity and Pipeline API HTTP reachability. Returns `HTTP 503` if any core dependency is down — shipped 2026-07-08.
|
||||
- **Ad-hoc** Connected `/scan-pfm` page with `/api/scan-pfm` route and enabled auto-trigger scanning on custom file upload, sample selection, thumbnail change, and canvas rotation. Supported both `image` and `image_base64` payload keys — shipped 2026-07-09.
|
||||
- **9.1** Added `GET /api/v1/documents/:id` (same 401/403 scoping as `PUT`), returning a single document — including still-unparsed rows — with a new `parseStatus: "pending"|"done"|"failed"` field, so the Flutter poller can move off scanning the entire list every 2s. Added `scan_mode`/`parse_error` columns to `documents` (`db/init.ts`, migrated via `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` for already-running DBs). `scan_mode` is now persisted on upload (`v1/documents/upload/route.ts`) and on the classic `/api/parse` route's upserts (`COALESCE`, same pattern as `kode_toko`), and surfaced as `docType` on every GET response (`utils/document-mapper.ts`, a new shared helper extracted from the list route's inline mapping so list/by-id/dedup all agree) — falling back to the legacy `order_untuk == "PRODUCT SCAN"` sentinel for pre-existing rows with no `scan_mode`. `parse_error` is now recorded when the upload route's *internal* call to `/api/parse` itself fails to complete (network error or the 210s abort firing) — previously this was silently swallowed and the document stayed `parsed=false` forever with no signal, burning the client's full 260s timeout; `/api/parse`'s own existing pipeline-error fallback (`parsed=true` + "Not Found" placeholder) was already fine and is unchanged. Also fixed the dedup branch (a repeat upload of an already-seen file) to return the original document's real current state via the same mapper instead of a hardcoded empty stub. Verified via `docker compose up -d --build` + `curl`: schema migration applied cleanly to the live DB (confirmed via `psql`), DO and Product uploads both correctly persist `scan_mode` and surface it as `docType`, a dedup retry returns real header/items instead of an empty stub, `GET /:id` returns 401 (no token) / 403 (wrong store) / 404 (nonexistent id) / 200 (admin or owning store), and the list endpoint's existing filter/scoping is unchanged — shipped 2026-07-10.
|
||||
- **9.3** Added authenticated `POST /api/v1/scan-product`, the v1 equivalent of the classic dev-only `/api/scan-pfm` (unauthenticated, and unreachable off-LAN since task 4.5 restricted the public tunnel to `/api/v1/*`). Extracted the shared classify-and-match logic (Python classifier call + Levenshtein SKU matching against `sku_master`, top-5 scoring) out of `api/scan-pfm/route.ts` into a new `utils/product-scan.ts` (`classifyAndMatchProduct`, plus a `ClassifierError` class that preserves forwarding the classifier's own HTTP status instead of collapsing every failure to 500) so the classic route and the new v1 route share one implementation instead of duplicating it — the classic route's response shape, auth-free behavior, and desktop-only layout-parsing visualization are otherwise unchanged. The new route accepts **either** multipart (`image`/`file` field, matching the v1 upload route's convention) or a JSON `{image_base64}` body, is open to any authenticated account (not admin-gated, since this is what the mobile app itself calls), and wraps the result in the standard `{status, data}` envelope with `classification`, `ocr` (including `extracted_expired_date`), and `possibleMatches`. Verified via `curl` against the live stack with a real product photo: multipart upload and JSON-body variants both return identical, correct top-5 matches; no-token request returns 401; the classic `/api/scan-pfm` route's response (including `layoutParsingResult`) is unchanged post-refactor — shipped 2026-07-10.
|
||||
- **9.2** Relaxed `GET /api/v1/master/skus` (`master/skus/route.ts`) so any authenticated account can read the SKU master list, not just `admin` — the Flutter product editor needs this and previously had to string-hack its base URL to call the unauthenticated classic `GET /api/skus`, which task 4.5 had already removed from the public tunnel, breaking product scans off-LAN. Changed the guard from a combined `!account || role !== 'admin'` check (403 for both "no token" and "wrong role") to `!account` (correct 401) followed by an unconditional pass-through for any valid account; `POST` (SKU creation) is untouched, still admin-only, per the user's explicit choice between the two options this task flagged as undecided. No response-shape change. Verified via `curl` against the live stack with a real non-admin (`store` role) account's token: `GET` → 200 with real data; no token → 401 (was incorrectly 403 before this fix); the same non-admin token against `POST` → still 403; admin `GET` → still 200. Along the way, hit and resolved a dev-loop issue: the container had the edited file on disk but Turbopack's file watcher wasn't detecting the change over the Windows bind mount, requiring `docker restart paddleocr-pfm-web-app` to pick it up — noted in case it recurs for future edits. With 9.1-9.3 all shipped, Flutter root task 7.1 (moving the product editor onto the v1 surface) is now fully unblocked — shipped 2026-07-10.
|
||||
|
||||
## Backend — OCR Pipeline & Accuracy
|
||||
|
||||
- **2.1** Built the Product/SKU scan classifier's model artifacts: `models/dinov2_index.pkl` (118/118 reference photos indexed across 16 SKU classes) and `models/produk-pfm-classifier-26n-100e-2026-07-08.pt` (+ `.onnx` export) — a YOLO classifier fine-tuned 100 epochs, 83.3% top-1 / 90% top-5 validation accuracy on the current (thin, 2-16 photos/class) dataset. Built via a one-off `docker run` from a freshly-rebuilt `pipeline-api` image (bare-metal training isn't viable on Windows — `paddlepaddle-gpu`'s wheel index is Linux-only). `pipeline-api` restarted and confirmed loading both models from logs. Also fixed `scripts/install-pipeline.sh`, which was missing `ultralytics`/`torch` — shipped 2026-07-08.
|
||||
- **2.1 (verification pass)** Ran a full browser walkthrough of `/scan-pfm` (classification, top-5, OCR expiry extraction + crop, SKU-master matching, Visual/Spotting Grid, Raw Response — all confirmed working with real data). Found and fixed a real bug: "Save Ground Truth" was returning success but silently writing into the `pfm-web-app` container's ephemeral filesystem instead of the host, because `/sources` wasn't a bind-mounted path in root `docker-compose.yml`. Added `./backend/sources:/sources` to the `pfm-web-app` service, recovered an orphaned entry via `docker cp`, and re-verified the save now persists to `backend/sources/product_manual_labels.json` on the host (confirmed the DO-flow's `manual_labels.json` save was fixed by the same change too) — shipped 2026-07-08.
|
||||
- **2.3** Ran the accuracy regression harness and discovered `sources/accuracy_report.md` was badly stale (claimed 75.04%; real current baseline is **95.10% overall, already at/above the 95% target** — added a staleness banner to that file). Root-caused every remaining mismatch by pulling raw OCR text from Postgres (`documents.layout_parsing_result`): the worst field, `plat` (67.6%), is almost entirely the license-plate region being classified as an image/seal by the layout model rather than OCR'd as text — not fixable in `parser.ts`. Found and fixed one genuine parser logic bug along the way: the "global pattern scanning fallback" could duplicate an already-correctly-extracted `noDO` value into a still-missing `noSO` field; fixed by excluding already-assigned values from that fallback's candidate pool (`pfm-web-app/src/utils/parser.ts`). Doesn't change the aggregate score (a wrong value and "Not Found" score the same) but stops a fabricated-looking wrong number from silently reaching the database. All 48 parser unit tests still pass — shipped 2026-07-08.
|
||||
- **Ad-hoc** Built custom expiry-date-based auto-rotation algorithm in Python classifier server (`classify_ocr_server.py`). The algorithm calculates the slant angle of the Expiry Date / Batch text line bounding box, automatically rotates the image to make it horizontal, and re-runs YOLO classification + PaddleOCR for maximum accuracy. Enhanced SKU matching database lookup to prioritize exact SKU matches with a score of 1.0, pinning them as the Best Match — shipped 2026-07-09.
|
||||
|
||||
## Backend — Postgres Data Layer
|
||||
|
||||
@@ -77,3 +82,12 @@ workflow and have no task numbers; see `git log` for real dates/history.
|
||||
- **7.1** Seeded one account per store in `db/init.ts` during initialization by assigning `username = kode_toko` and a bcrypt-hashed default password `"123"`. Included `role` and `is_active` schema additions — shipped 2026-07-08.
|
||||
- **7.2** Enhanced authentication routing by modifying `POST /api/v1/auth/login` to perform a `LEFT JOIN` on `store_master`, returning the extended store profile alongside the token. Added a guard to reject login if `is_active = false`. Implemented a new `GET /api/v1/auth/me` endpoint to cleanly re-fetch the profile via token — shipped 2026-07-08.
|
||||
- **7.3** Created a reproducible `store_master` bootstrap logic in `db/init.ts` that reads from `sources/toko_aktif.json` idempotently on startup. Also correctly seeded the `WH_JOFFICE` head office to resolve the admin account foreign-key setup constraint — shipped 2026-07-08.
|
||||
|
||||
## Backend — Document Confirmation Gate & Data Hygiene
|
||||
|
||||
- **10.1** Added a `confirmed BOOLEAN NOT NULL DEFAULT TRUE` column to `documents` (`db/init.ts`, `ALTER TABLE ... ADD COLUMN IF NOT EXISTS` — grandfathers every pre-existing row so today's history didn't go empty after migration) and used it to separate "OCR finished" from "user confirmed": previously `GET /api/v1/documents` filtered only on `parsed = true`, which the backend sets synchronously right after upload — before the mobile user ever taps "Simpan & Konfirmasi" in the editor — so a scan captured, previewed, then backed out of (never confirmed) was already sitting in every entitled account's document list with blank/placeholder fields (root cause of `document_card.dart`'s "Staff Toko" fallback text on the Flutter side). `v1/documents/upload/route.ts` now explicitly inserts `confirmed = false` on every new upload; `v1/documents/[id]/route.ts`'s `PUT` handler is the *only* place that flips it to `true` (literally "the user confirmed"); `v1/documents/route.ts` (list) now filters `AND confirmed = true` unconditionally for every account including `admin` (no role special-casing, per explicit user decision); `v1/documents/[id]/route.ts`'s `GET`-by-id handler is deliberately untouched by the new filter so the mobile poller can keep seeing pending/unconfirmed documents mid-flow. `utils/document-mapper.ts`'s shared `DocumentRow`/`mapDocumentRow()` now carries `confirmed` through to all three call sites (list, GET-by-id, upload's dedup-hit branch) from one place. `parse/route.ts`'s own `INSERT ... ON CONFLICT (filename) DO UPDATE` statements (both DO and Product branches) were deliberately left untouched for `confirmed` — in the real mobile flow the upload route's INSERT always runs first, so this upsert always hits the `ON CONFLICT` branch, and since its `SET` clause doesn't mention `confirmed`, Postgres correctly leaves the existing value alone (verified this is correct, not an oversight). Verified live against the running Docker stack: uploaded a real DO photo as a store account without confirming it — absent from that store's list (and from `admin`'s) while `GET /documents/:id` still reported the correct `parseStatus`; `PUT` (confirm) made it appear immediately with the real submitted data; all 13 pre-existing rows carried `confirmed = true` after the migration ran — shipped 2026-07-10.
|
||||
- **10.2** Removed the fabricated Product Scan placeholder values `noPO: "PO-PRODUCT-001"`, `noSO: "1002003004"`, `noDO: "DO-PRODUCT-999"` (both the flat keys and the mirrored `header.no_po`/`no_so`/`no_do` sub-object) from `parse/route.ts`'s Product-scan branch, replacing them with empty strings — these are DO-specific concepts that don't apply to a product verification scan, and were never actually read by anything: `pdf_service.dart`'s Product receipt branch never prints them, and `product_editor_submit_logic.dart`'s `_submit()` builds its own `noPo`/`noSo`/`noDo` from the user's PO-link dropdown and batch selection, ignoring the stored values entirely. Same class of issue as the earlier G7 fix (fabricated data presented as if real) — low risk to remove since nothing meaningfully depended on the old values. Scope stayed narrow to exactly these three fields; `nama_driver`/`nama_penerima`'s "PRODUCT SCAN"/"STORE STAFF" placeholders were left alone as a deliberate fixed convention, not a fabricated document number. Verified via `curl`: a freshly-uploaded, unconfirmed Product Scan document's raw `GET /documents/:id` response now returns `no_po`/`no_so`/`no_do` as empty strings instead of the old fake values — shipped 2026-07-10.
|
||||
|
||||
## Backend — Single-Pass Product Classification
|
||||
|
||||
- **11.1** Eliminated the duplicate GPU classification pass on Product Scan (gap G3), sourced from user feedback that the review screen took noticeably longer to open than DO Scan's. `api/parse/route.ts`'s Product branch previously had its own separate, poorer inline classify call (kept only `top1_name`/`extracted_sku`), forcing the Flutter editor to re-run the entire classify+OCR pipeline a second time via `POST /api/v1/scan-product` just to get the top-5 candidate list and OCR-extracted expiry date. Now calls the same shared `classifyAndMatchProduct()` (`utils/product-scan.ts`) already used by that v1 route — one GPU call, richer result — and persists it under a new `metadata.productScan` JSONB key (no schema migration), surfaced by `document-mapper.ts` as a top-level `productScan` field on every GET response. Caught and fixed a real regression along the way: delegating to the shared function silently dropped the 90s pipeline timeout the old inline fetch had; added the same bound (`PIPELINE_TIMEOUT_MS`) directly inside `classifyAndMatchProduct()` so both callers — this route and the live `POST /api/v1/scan-product` (which never had the bound either) — are protected. Verified via `curl` with a genuinely fresh image/store combination (proving a real classify pass, not a dedup hit): took 9s, and the immediate `GET /documents/:id` response already contained 5 real `possibleMatches` and the extracted expiry date, before any editor interaction — shipped 2026-07-10.
|
||||
@@ -105,3 +105,387 @@ Conduct a code review and audit of the implementations for Tasks 6.1-6.3 (Ground
|
||||
|
||||
## 4. Conclusion
|
||||
The Product Scan Ground Truth Annotation and Evaluation tools operate perfectly. The system can now durably store base64 test images, manually correct AI anomalies, and automatically evaluate retrained models with historical tracking. Documentation drift has been comprehensively resolved. No regressions were observed.
|
||||
|
||||
---
|
||||
|
||||
# Iteration Log & Audit: Flutter Client Contract, Server Half (Task 9.1)
|
||||
|
||||
## 1. Objective
|
||||
Conduct a code review and audit of task 9.1 — `GET /api/v1/documents/:id` with an
|
||||
explicit `parseStatus`, and `scan_mode` persistence surfaced as `docType` — to close
|
||||
gaps G1/G10/G4 (server half) documented in `docs/api-contract-map.md`.
|
||||
|
||||
## 2. Code Review
|
||||
|
||||
### 2.1 Schema (`pfm-web-app/src/db/init.ts`)
|
||||
- New `scan_mode VARCHAR(20)` / `parse_error TEXT` columns added to both the
|
||||
`CREATE TABLE IF NOT EXISTS` body and an `ALTER TABLE ... ADD COLUMN IF NOT
|
||||
EXISTS` migration line, matching the exact pattern already used for `kode_toko` —
|
||||
safe to run against an already-populated production DB without downtime.
|
||||
|
||||
### 2.2 Shared mapper (`pfm-web-app/src/utils/document-mapper.ts`, new file)
|
||||
- Extracted the header/shipment branch-mapping logic (`metadata.header` present vs.
|
||||
legacy web-parser shape) that previously only lived inline in the list route, so
|
||||
the new GET-by-id route and the upload route's dedup-response branch can't drift
|
||||
from the list route's mapping. Computes `parseStatus` from `parsed`/`parse_error`
|
||||
and `docType` from `scan_mode`, falling back to the legacy `order_untuk ==
|
||||
"PRODUCT SCAN"` sentinel for rows predating this column — verified via `curl`
|
||||
against a pre-existing pre-9.1 document that it doesn't regress to `docType:
|
||||
undefined`.
|
||||
|
||||
### 2.3 `GET /api/v1/documents/:id` (`api/v1/documents/[id]/route.ts`)
|
||||
- Reuses the exact same auth/scoping pattern as the existing `PUT` on the same
|
||||
file (401 no-account, 404 no-row, 403 non-admin/wrong-store) — no new auth
|
||||
logic introduced, just the existing helper called a second time.
|
||||
- Deliberately omits the list route's `parsed = true` filter, since surfacing
|
||||
pending/failed rows is the entire point of the endpoint.
|
||||
|
||||
### 2.4 Failure recording (`api/v1/documents/upload/route.ts`)
|
||||
- The one gap not already covered by `/api/parse`'s own pre-existing error
|
||||
fallback (which already flips `parsed=true` with "Not Found" placeholder
|
||||
metadata, unchanged by this task) is the internal fetch call to `/api/parse`
|
||||
itself never completing — network error or the pre-existing 210s
|
||||
`AbortSignal.timeout` firing. Both that `catch` branch and a new `!response.ok`
|
||||
check now persist a short message to `documents.parse_error`, which is the only
|
||||
input the new `parseStatus: "failed"` branch depends on.
|
||||
- Dedup branch fixed to run the existing document through the same shared mapper
|
||||
instead of a hand-built always-empty stub (G10) — a GPS-tag fallback to the
|
||||
retry's own coordinates was preserved for documents that never got one on first
|
||||
upload, matching the previous behavior's intent.
|
||||
|
||||
### 2.5 `scan_mode` persistence in `/api/parse` (`api/parse/route.ts`)
|
||||
- Minimal, additive `COALESCE(EXCLUDED.scan_mode, documents.scan_mode)` in both
|
||||
`ON CONFLICT` blocks, same pattern already used for `kode_toko` — so documents
|
||||
created via the classic route (not just the v1 upload path) also get a correct
|
||||
`scan_mode`. `parse_error = NULL` added to both `SET` clauses to clear a stale
|
||||
failure once a parse actually completes. This file remains accepted §B3 debt
|
||||
(604 lines pre-existing, per `backend/AGENTS.md` Adaptation Notes) — touched only
|
||||
minimally, not restructured, consistent with that note's "split only if/when
|
||||
touched" guidance being about restructuring, not about refusing small edits.
|
||||
|
||||
## 3. Audit Verification
|
||||
- **Functional testing** (`docker compose up -d --build` from repo root, real
|
||||
`curl` calls against the live stack, not just unit tests):
|
||||
- Confirmed `scan_mode`/`parse_error` columns exist post-migration via `psql \d
|
||||
documents` against the running container — no `ALTER TABLE` errors in logs.
|
||||
- Logged in as a real store account (`WH_JCIBBR1`), uploaded a real DO test
|
||||
image (`sources/test-images/do-001.jpg`): `GET /api/v1/documents/:id` returned
|
||||
the real parsed header/items, `parseStatus: "done"`, `docType: "DO"`.
|
||||
- Re-uploaded the identical file (dedup path): response now carries the same
|
||||
real header/items instead of the old empty stub — confirmed G10 fixed.
|
||||
- Uploaded a real product photo with `scan_mode=Product`: `docType: "Product"`
|
||||
confirmed both in the GET response and directly in Postgres
|
||||
(`SELECT scan_mode FROM documents`).
|
||||
- `GET /:id` with no token → 401; nonexistent id → 404; a *different* store
|
||||
account's token against another store's document → 403; `admin`'s token
|
||||
against the same document → 200 (admin bypass intact).
|
||||
- `GET /api/v1/documents` (list) still returns only parsed, non-sample
|
||||
documents, now carrying `docType`/`parseStatus` for free via the shared
|
||||
mapper — existing 401 behavior unchanged.
|
||||
- `npx tsc --noEmit` clean across the whole `pfm-web-app` project.
|
||||
|
||||
## 4. Conclusion
|
||||
Task 9.1 closes gaps G1 (no per-document GET / N+1 list polling), G10 (dedup stub),
|
||||
and the server half of G4 (fabricated doc-type sentinel) exactly as scoped. All
|
||||
new behavior was verified against the live Docker stack with real uploads, not
|
||||
just a clean build — auth/scoping regressions were explicitly checked and none
|
||||
were found. Flutter-side consumption (`plans/next-enhancements.md` §6.1/§7.3)
|
||||
remains open and unblocked by this change.
|
||||
|
||||
---
|
||||
|
||||
# Iteration Log & Audit: Authenticated v1 Product-Scan Endpoint (Task 9.3)
|
||||
|
||||
## 1. Objective
|
||||
Conduct a code review and audit of task 9.3 — `POST /api/v1/scan-product`, an
|
||||
authenticated equivalent of the classic dev-only `/api/scan-pfm` — to close gap
|
||||
G2/G3 (`docs/api-contract-map.md`): the Flutter product editor currently reaches
|
||||
the classify+match pipeline via an unauthenticated route that task 4.5 already
|
||||
excluded from the public tunnel, so product scanning is broken off-LAN.
|
||||
|
||||
## 2. Code Review
|
||||
|
||||
### 2.1 Shared util (`pfm-web-app/src/utils/product-scan.ts`, new file)
|
||||
- `classifyAndMatchProduct` is a byte-for-byte extraction of the classic route's
|
||||
classify-call + Levenshtein-SKU-match logic (not a rewrite) — reduces the risk
|
||||
that the new v1 route's behavior silently diverges from the already-working
|
||||
classic route's matching quality.
|
||||
- `ClassifierError` deliberately preserves the classic route's existing behavior
|
||||
of forwarding the Python classifier's own HTTP status on failure, rather than
|
||||
letting a generic `catch` collapse every failure to 500 — both the classic and
|
||||
new v1 route special-case it identically.
|
||||
- Intentionally excludes the layout-parsing visualization block: that's
|
||||
desktop-test-page-only per the task's explicit response-field list
|
||||
(`possibleMatches`, `ocr`, `classification` — no `layoutParsingResult`), so it
|
||||
correctly stays in `api/scan-pfm/route.ts` rather than being pulled into the
|
||||
shared util or the new v1 route.
|
||||
|
||||
### 2.2 Classic route refactor (`api/scan-pfm/route.ts`)
|
||||
- Response shape (`{classification, ocr, possibleMatches, layoutParsingResult}`,
|
||||
no envelope, no auth) is unchanged — this route still serves the desktop test
|
||||
page exactly as before, now just calling the shared util instead of inlining
|
||||
the logic. Dropped one genuinely dead variable (`extractedProductName`, computed
|
||||
but never read in the original code) as part of the extraction.
|
||||
|
||||
### 2.3 New v1 route (`api/v1/scan-product/route.ts`)
|
||||
- Auth: any authenticated account (not admin-gated) — correct, since this is the
|
||||
route the mobile app's own store-role accounts call to perform a scan, unlike
|
||||
`master/skus` writes which are intentionally admin-only.
|
||||
- Dual input handling (multipart primary, JSON base64 fallback) matches the task's
|
||||
explicit wording ("multipart (preferred...) or base64") and lets Flutter adopt
|
||||
this endpoint today regardless of which shape task 7.1 ends up sending.
|
||||
- No `nginx.conf` change was needed — confirmed the port-8001 restricted block's
|
||||
`location /api/v1/` (line 135) is a prefix match already covering the new path.
|
||||
|
||||
## 3. Audit Verification
|
||||
- **Functional testing** against the live stack (same running containers as task
|
||||
9.1's session; `pfm-web-app` restarted once to pick up the new route file after
|
||||
its dev-server file watcher missed the new directory — a known bind-mount
|
||||
quirk on Windows Docker Desktop, not a code issue):
|
||||
- `POST /api/v1/scan-product` with a real product photo as multipart `image` +
|
||||
a real store account's bearer token: `200`, `{status:"success", data:
|
||||
{classification, ocr, possibleMatches}}` with a correct top-5 match list and
|
||||
`isBestMatch` on the top entry; `ocr` confirmed to include
|
||||
`extracted_expired_date`.
|
||||
- Same call with no token → `401`.
|
||||
- Same image via a JSON `{image_base64}` body instead of multipart → identical
|
||||
`possibleMatches` output, confirming both input paths produce the same result.
|
||||
- Classic `POST /api/scan-pfm` (JSON body, no auth) with the same image →
|
||||
unchanged response shape and matching results, including
|
||||
`layoutParsingResult` still present — no regression from the extraction.
|
||||
- `npx tsc --noEmit` and `npx eslint` on the three touched/new files clean
|
||||
(aside from pre-existing `any`-for-JSONB-shaped-data style already used
|
||||
throughout this codebase, e.g. `document-mapper.ts` from task 9.1).
|
||||
|
||||
## 4. Conclusion
|
||||
Task 9.3 closes gap G2/G3 exactly as scoped: the mobile app now has an
|
||||
authenticated, tunnel-reachable path to the classify+match pipeline that returns
|
||||
identical results to the already-proven classic route, verified against real
|
||||
classifier output rather than mocked data. Task 9.2 (non-admin SKU list read)
|
||||
remains open and separate. Flutter-side consumption (`plans/next-enhancements.md`
|
||||
§7.1) remains open and is now unblocked by this change (alongside 9.2).
|
||||
|
||||
# Iteration Log & Audit: Non-Admin SKU List Read (Task 9.2)
|
||||
|
||||
## 1. Objective
|
||||
Conduct a code review and audit of task 9.2 — read access to the SKU master
|
||||
list (`GET /api/v1/master/skus`) for any authenticated account, not just
|
||||
`admin` — closing gap G2 alongside 9.3. Picked up via an explicit
|
||||
backend-scoped `n{9.2}` request after the user asked to clarify the two
|
||||
options the task itself flagged as undecided (relax the existing endpoint
|
||||
vs. add a new one); user chose to relax the existing endpoint.
|
||||
|
||||
## 2. Code Review
|
||||
- `master/skus/route.ts`'s `GET` handler previously rejected any non-`admin`
|
||||
account with 403, forcing the Flutter product editor to call the
|
||||
unauthenticated classic `GET /api/skus` instead (the actual bug this task
|
||||
fixes - that classic route was removed from the public ngrok tunnel by
|
||||
task 4.5, so product scans off-LAN were already broken before this fix).
|
||||
- Changed the `GET` guard from `!account || account.role !== 'admin'`
|
||||
(403 either way) to `!account` (401 for no/invalid token, any valid
|
||||
account now passes) - a one-line, surgical change matching the user's
|
||||
chosen option exactly. `POST` (SKU creation) was deliberately left
|
||||
untouched, still admin-gated - the task's own text specified "writes stay
|
||||
admin-only," and admin master-data management is a different concern from
|
||||
a mobile client reading the catalog to populate a dropdown.
|
||||
- No response-shape change: still `{status: "success", data: res.rows}`,
|
||||
matching what task 9.2 asked for (the `{status, data}` v1 envelope) and
|
||||
what the Flutter product editor already expects once it switches over
|
||||
(root task 7.1, not yet picked up).
|
||||
|
||||
## 3. Audit Verification
|
||||
- Hit a real hot-reload gap during verification: the file was correctly
|
||||
updated on disk inside the `pfm-web-app` container (confirmed via
|
||||
`docker exec ... cat`), but the running Turbopack dev server kept serving
|
||||
the old admin-gated behavior - a known class of issue where Windows-host
|
||||
bind-mount file-change events don't reliably reach `next dev`'s watcher.
|
||||
Fixed by `docker restart paddleocr-pfm-web-app`, after which the new code
|
||||
took effect immediately (confirmed via a fresh `curl` round-trip).
|
||||
- Verified via `curl` against the live stack (real account credentials
|
||||
pulled from the live `accounts` table, not fixtures):
|
||||
- A real non-admin (`store` role) account's token: `GET
|
||||
/api/v1/master/skus` → `200`, real `sku_master` rows returned.
|
||||
- No `Authorization` header at all: `401 Unauthorized` (previously this
|
||||
same case incorrectly returned `403`, since the old guard checked
|
||||
`!account || role !== 'admin'` as one combined condition - now correctly
|
||||
distinguishes "no valid account" from "valid but insufficient role").
|
||||
- The same non-admin token against `POST /api/v1/master/skus` (attempting
|
||||
to create a SKU): still `403 Forbidden: Admin access required` - writes
|
||||
unaffected.
|
||||
- An admin token against `GET /api/v1/master/skus`: still `200` - no
|
||||
regression for the existing admin master-data UI.
|
||||
|
||||
## 4. Conclusion
|
||||
Task 9.2 closes gap G2's remaining half: the mobile app can now read the
|
||||
SKU master list through the authenticated, tunnel-reachable `/api/v1/*`
|
||||
surface without impersonating a dev-only unauthenticated route. Combined
|
||||
with 9.1 and 9.3 (both already shipped), every backend blocker behind root
|
||||
`plans/next-enhancements.md` §7.1 (moving the product editor onto the v1
|
||||
surface) is now cleared - that Flutter task is unblocked and ready to pick
|
||||
up. §7.2 (eliminating the duplicate classification pass) is separately
|
||||
unblocked in principle (9.1's `docType`/metadata work + 9.3's endpoint both
|
||||
exist now) but still needs its own client-side decision about which single
|
||||
pass to keep, per that task's own grill-me note.
|
||||
|
||||
---
|
||||
|
||||
## Iteration & Audit: Tasks 10.1/10.2 — Document Confirmation Gate & Data Hygiene (2026-07-10)
|
||||
|
||||
### 1. Objective
|
||||
Close backend §10, sourced from user feedback on the release APK
|
||||
(`twinkly-riding-mitten.md`, root-cause documented as gaps **G11**/**G12** in
|
||||
`docs/api-contract-map.md`): documents were visible via `GET /api/v1/documents`
|
||||
the instant OCR parsing finished, before the mobile user ever confirmed them
|
||||
via `PUT`, and Product Scan uploads carried fabricated `noPO`/`noSO`/`noDO`
|
||||
placeholder values.
|
||||
|
||||
### 2. Code Review
|
||||
- **`db/init.ts`**: new `confirmed BOOLEAN NOT NULL DEFAULT TRUE` column, both
|
||||
in the `CREATE TABLE IF NOT EXISTS` block and as an idempotent
|
||||
`ALTER TABLE ... ADD COLUMN IF NOT EXISTS` for already-running DBs, matching
|
||||
the exact pattern already used for `scan_mode`/`parse_error`. `DEFAULT TRUE`
|
||||
is a deliberate grandfather clause — every row that existed before this
|
||||
migration counts as already-confirmed, so existing history doesn't vanish.
|
||||
- **`v1/documents/upload/route.ts`**: the one real INSERT path for a fresh
|
||||
mobile capture now explicitly inserts `confirmed = false`; the dedup-hit
|
||||
branch (no INSERT) is untouched, correctly reflecting whatever state the
|
||||
original row already has. Its SELECT for the dedup branch was also extended
|
||||
to fetch `confirmed` so the mapper has it.
|
||||
- **`utils/document-mapper.ts`**: `DocumentRow` interface and
|
||||
`mapDocumentRow()`'s return both carry `confirmed` through now, so all three
|
||||
call sites (list, GET-by-id, upload dedup) stay in sync from one place —
|
||||
same shared-mapper pattern task 9.1 established.
|
||||
- **`v1/documents/route.ts`** (list): `AND confirmed = true` added to the
|
||||
`WHERE` clause with no role branching — applies to `admin` exactly the same
|
||||
as `store` accounts, per the user's explicit answer when asked whether admin
|
||||
should retain oversight visibility into unconfirmed documents (they chose
|
||||
"no special-casing").
|
||||
- **`v1/documents/[id]/route.ts`**: `PUT` now sets `confirmed = true` alongside
|
||||
the existing `parsed = true` in its `UPDATE` — the *only* place this flips.
|
||||
`GET`-by-id is untouched, deliberately: its existing comment already says
|
||||
the point of this endpoint is letting the poller see pending/failed
|
||||
documents, and that reasoning extends unchanged to unconfirmed ones — the
|
||||
poller must detect parse-completion before the user has had a chance to
|
||||
confirm anything.
|
||||
- **`parse/route.ts`**: reasoned through, rather than blindly copied, whether
|
||||
its own `INSERT ... ON CONFLICT (filename) DO UPDATE` statements (DO and
|
||||
Product branches) needed `confirmed` handling. In the real mobile flow the
|
||||
upload route's INSERT always runs first, so this statement always resolves
|
||||
via the `ON CONFLICT` branch; since `confirmed` is absent from that branch's
|
||||
`SET` clause, Postgres leaves the row's existing value untouched by design —
|
||||
correct behavior (never regress an already-confirmed row, never reset a
|
||||
pending one mid-reparse) without adding a single line. Also removed the
|
||||
Product branch's fabricated `noPO`/`noSO`/`noDO` placeholder values (task
|
||||
10.2) — replaced with empty strings after confirming (by reading
|
||||
`pdf_service.dart` and `product_editor_submit_logic.dart` on the Flutter
|
||||
side) that nothing reads them meaningfully; the confirmed document's real
|
||||
values always come from the user's own PO-link/batch selection at PUT time
|
||||
regardless.
|
||||
|
||||
### 3. Audit Verification
|
||||
Live against the running Docker stack (`docker restart paddleocr-pfm-web-app`
|
||||
to pick up the code + run the migration):
|
||||
- `\d documents` confirmed the new `confirmed boolean not null default true`
|
||||
column; `SELECT count(*) FROM documents WHERE confirmed = true` returned
|
||||
13 (all pre-existing rows), `= false` returned 0 — grandfather clause held.
|
||||
- Uploaded a real DO photo as store account `WH_JCIBBR1` without ever calling
|
||||
`PUT`: absent from that store's `GET /documents` (count unchanged at 3, new
|
||||
id 3400 not present) and absent from `admin`'s list too (13, unchanged);
|
||||
`GET /documents/3400` still returned `parseStatus: "done"`,
|
||||
`confirmed: false` — the poller/editor hand-off path is unaffected.
|
||||
- `PUT /documents/3400` (confirm) with real header/shipment data: doc count
|
||||
for `WH_JCIBBR1` became 4, id 3400 now present with the real submitted
|
||||
`namaPenerima` ("Penerima Test", not a placeholder); `psql` confirmed the
|
||||
row's `confirmed` column flipped to `t`.
|
||||
- Uploaded a fresh Product Scan as the same store (doc id 3402), read its raw
|
||||
unconfirmed `GET /documents/3402` response: `header.no_po`/`no_so`/`no_do`
|
||||
all returned `""` — the old `"PO-PRODUCT-001"`/`"1002003004"`/
|
||||
`"DO-PRODUCT-999"` placeholders are gone.
|
||||
|
||||
### 4. Conclusion
|
||||
Backend §10 is fully `[DONE]`. Flutter's corresponding root task §8.2 (add an
|
||||
optional `confirmed` field to `DocumentModel`, default `true` for
|
||||
legacy/cached responses) was implemented and verified in the same session —
|
||||
see root `docs/iteration-log.md`. No remaining backend blocker for gap G11 or
|
||||
G12.
|
||||
|
||||
---
|
||||
|
||||
## Iteration & Audit: Task 11.1 — Single-Pass Product Classification (2026-07-10)
|
||||
|
||||
### 1. Objective
|
||||
Close backend §11 (gap **G3**), sourced directly from user feedback after
|
||||
they noticed Product Scan's confirmation screen took visibly longer to open
|
||||
than DO Scan's and asked why. G3 had been documented earlier this session in
|
||||
`docs/api-contract-map.md` but deliberately left `[TODO]`/deferred, pending
|
||||
exactly the client-side decision the user's follow-up message resolved:
|
||||
"sama seperti scan DO... GPU tidak 2x kerja" (same as DO scan, GPU shouldn't
|
||||
run twice) — i.e. do the classify pass once, at upload, and have the editor
|
||||
read the stored result, not re-classify on review.
|
||||
|
||||
### 2. Code Review
|
||||
- **`api/parse/route.ts`'s Product branch**: replaced its own separate,
|
||||
inline `fetch(pyServerUrl, ...)` (which discarded everything except
|
||||
`top1_name`/`extracted_sku`) with a call to the already-existing shared
|
||||
`classifyAndMatchProduct()` from `utils/product-scan.ts` — the same
|
||||
function `POST /api/v1/scan-product` (task 9.3) uses, which additionally
|
||||
runs the Levenshtein SKU-match against `sku_master` for a real top-5
|
||||
candidate list and returns the raw OCR result (`extracted_expired_date`
|
||||
included). `b64` (the image's base64 encoding) was already computed
|
||||
earlier in this function for the DO path — reused, not recomputed, so
|
||||
this is a strict reduction in duplicated work, not an addition.
|
||||
- **New `metadata.productScan` key**: `{ possibleMatches, extractedExpiryDate
|
||||
}` stored alongside the existing `header`/`shipment`/`items` keys in the
|
||||
same JSONB `metadata` column — no migration, following the exact precedent
|
||||
those other keys already set for coexisting shapes in one column.
|
||||
- **`utils/document-mapper.ts`**: added a top-level `productScan` field to
|
||||
`mapDocumentRow()`'s return (`metadata.productScan || null`), so all three
|
||||
GET call sites (list, by-id, upload dedup) expose it identically, same
|
||||
shared-mapper pattern as `parseStatus`/`docType`/`confirmed`.
|
||||
- **Regression audit, not just addition**: read `classifyAndMatchProduct()`'s
|
||||
own `fetch` call closely while wiring it into `parse/route.ts` and noticed
|
||||
it had *no* `AbortSignal` at all — the inline call it was replacing in
|
||||
`parse/route.ts` had an explicit 90s bound
|
||||
(`PIPELINE_TIMEOUT_MS`/`AbortSignal.timeout`). Silently dropping that bound
|
||||
would have been a real regression (a wedged GPU container hanging past the
|
||||
intended fail-fast point). Fixed by adding the identical 90s bound directly
|
||||
inside `classifyAndMatchProduct()` itself — which also retroactively fixes
|
||||
the live `POST /api/v1/scan-product` route, which never had this bound
|
||||
either (pre-existing gap, not something this task's own diff introduced,
|
||||
but caught and closed while in the area).
|
||||
|
||||
### 3. Audit Verification
|
||||
Live against the running Docker stack (`docker restart paddleocr-pfm-web-app`
|
||||
to pick up the code):
|
||||
- Deliberately chose a genuinely fresh image/store combination
|
||||
(`do-015.jpg`, never uploaded before, as store `WH_JAFATAH`) to rule out a
|
||||
dedup hit masking whether real classification ran. Response was
|
||||
`"Document uploaded successfully"` (the fresh-insert branch, not the
|
||||
dedup-return branch) and took **9 seconds** — consistent with one real GPU
|
||||
classify+match pass, not a cache hit.
|
||||
- Immediate `GET /documents/:id` (no editor interaction, no second request)
|
||||
returned a fully populated `productScan`: 5 real `possibleMatches` with
|
||||
real `sku_master` names/scores (e.g. `"CHAMP CRUNCHY HOTZZ 300 GR/PAC"` at
|
||||
`score: 0.7575...`, matching real product naming conventions, not
|
||||
fabricated placeholders) and `extractedExpiryDate` (empty string here,
|
||||
since this particular test image has no visible expiry text - correctly
|
||||
reflecting a real "not found" rather than a fake date, consistent with the
|
||||
G7 fix's "no dummy data" rule).
|
||||
- Confirmed via a second, earlier check (before switching to the guaranteed-
|
||||
fresh combination above) that a dedup-hit response for a different
|
||||
document (id 3408) *also* returned a fully populated `productScan` from a
|
||||
prior parse - proving the data survives the dedup-return code path too
|
||||
(`upload/route.ts`'s dedup SELECT was already extended for `confirmed` in
|
||||
task 10.1's session and needed no further change here, since it maps
|
||||
through the same shared `mapDocumentRow()`).
|
||||
|
||||
### 4. Conclusion
|
||||
Backend §11 is `[DONE]`. Flutter's corresponding root task §7.2 (read
|
||||
`productScanMatches`/`productScanExtractedExpiryDate` directly from the
|
||||
document instead of re-calling `/scan-product`) was implemented and verified
|
||||
in the same session — see root `docs/iteration-log.md`. Gap G3 is resolved;
|
||||
`docs/api-contract-map.md` updated accordingly. `POST /api/v1/scan-product`
|
||||
itself is intentionally left in place (unused by this flow now, but a
|
||||
legitimate, reusable authenticated endpoint - e.g. for a possible future
|
||||
"rescan this photo" action) rather than removed, since removing a working,
|
||||
independently-useful route wasn't part of what this task's scope required.
|
||||
Reference in new issue
Block a user