Review follow-ups: PR #596 — fix(import-web): clear the web CSV-import UX blockers (browser pass 2026-09-25) #839

New Issue

2026-09-25T23:41:11Z

gambit-admin commented

2026-09-25 23:41:11 +00:00

Non-blocking findings from the auto code review of PR #596 (fix(import-web): clear the web CSV-import UX blockers (browser pass 2026-09-25)) — captured for batch cleanup rather than blocking the PR.
Commit 120f7f3 · https://github.com/Gambit-Inc/gambit/pull/596

SHOULD-FIX:

backend/app/routes/imports.py:2417: The reopen guard reads _row_lane(r) before r.decision is changed. A row with match_status == "auto" that was set aside (decision == "park") reports lane "parked", so the guard lets it through. After decision = "pending" its lane becomes "auto", which is not in _PENDING_LANES and stages nothing (_staging_candidate excludes pending). That is the silent drop the comment says the guard prevents. The UI never offers Undo on park rows, but the API accepts it directly. Guard on r.match_status == "auto" (and no blocking warning), or compute the lane as it would be after the change.
frontend/app/hq/imports/_components/DecidedRows.jsx:82: sugg[pick][1] throws a TypeError if picks[row_id] is out of range for the current sugg. The picks state keeps the index per row_id, and row.candidates can change after a reload or re-match. The select then shows a blank value, and "Use this" crashes the component. Clamp with sugg[Math.min(pick, sugg.length - 1)].
frontend/app/hq/imports/_components/DecidedRows.jsx:25: load has no stale-response guard. useEffect refires on every session object change, and refreshSession runs after most actions. An older, slower walkDecided reply can overwrite a newer one. A row the seller just undid can then reappear as "Decided" until the next refresh. Track a request token, as the resume runs do.
backend/app/routes/imports.py:2417: reopen clears decided_candidate but leaves match_signal = "operator-assigned" (set by assign) on the row. Confirm nothing downstream treats that signal as a lane or gate input once the row is pending again.

NITS:
none

Filed from a Claude Code session

Non-blocking findings from the auto code review of **PR #596** (fix(import-web): clear the web CSV-import UX blockers (browser pass 2026-09-25)) — captured for batch cleanup rather than blocking the PR. Commit `120f7f3` · https://github.com/Gambit-Inc/gambit/pull/596 SHOULD-FIX: - `backend/app/routes/imports.py:2417`: The reopen guard reads `_row_lane(r)` before `r.decision` is changed. A row with `match_status == "auto"` that was set aside (`decision == "park"`) reports lane `"parked"`, so the guard lets it through. After `decision = "pending"` its lane becomes `"auto"`, which is not in `_PENDING_LANES` and stages nothing (`_staging_candidate` excludes pending). That is the silent drop the comment says the guard prevents. The UI never offers Undo on park rows, but the API accepts it directly. Guard on `r.match_status == "auto"` (and no blocking warning), or compute the lane as it would be after the change. - `frontend/app/hq/imports/_components/DecidedRows.jsx:82`: `sugg[pick][1]` throws a TypeError if `picks[row_id]` is out of range for the current `sugg`. The `picks` state keeps the index per `row_id`, and `row.candidates` can change after a reload or re-match. The select then shows a blank value, and "Use this" crashes the component. Clamp with `sugg[Math.min(pick, sugg.length - 1)]`. - `frontend/app/hq/imports/_components/DecidedRows.jsx:25`: `load` has no stale-response guard. `useEffect` refires on every `session` object change, and `refreshSession` runs after most actions. An older, slower `walkDecided` reply can overwrite a newer one. A row the seller just undid can then reappear as "Decided" until the next refresh. Track a request token, as the resume runs do. - `backend/app/routes/imports.py:2417`: `reopen` clears `decided_candidate` but leaves `match_signal = "operator-assigned"` (set by `assign`) on the row. Confirm nothing downstream treats that signal as a lane or gate input once the row is pending again. NITS: none --- Filed from a Claude Code session

gambit-admin added the enhancement label 2026-09-25 23:41:11 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gambit/gambit#839