Review follow-ups: PR #670 — perf(import): review queue keys ~90ms → ~3ms; flag big price gaps (Check price) #903

New Issue

2026-09-30T20:20:04Z

gambit-admin commented

2026-09-30 20:20:04 +00:00

Non-blocking findings from the auto code review of PR #670 (perf(import): review queue keys ~90ms → ~3ms; flag big price gaps (Check price)) — captured for batch cleanup rather than blocking the PR.
Commit 31479db · https://github.com/Gambit-Inc/gambit/pull/670

SHOULD-FIX:

frontend/app/hq/imports/page.js:1705-1707 — ParkedPanel now owns its own local picks state instead of sharing the wizard-level picks passed down from ImportWizard/TriageStep. Previously the same picks object (keyed by row_id) was shared across the queue and the Parked panel, so a candidate index chosen while a row was still in the queue carried over once the row was set aside. Now that selection silently resets to index 0 the moment a row lands in Parked, because the new local useState({}) starts empty. Not data-destructive (nothing commits until the seller picks again), but it's an unannounced UX regression from the pre-refactor behavior.

NITS:

frontend/app/hq/imports/page.js:1784 vs :1818 — the new top-level const setPick = useCallback((id, i) => ...) and the pre-existing keyboard handler's local const setPick = (i) => ... inside onKey have the same name but different signatures (row-id-based vs index-based). The inner one shadows the outer; functionally harmless but confusing to read/lint.
frontend/app/hq/imports/_components/QueueRow.jsx:33 — containIntrinsicSize: "auto 80px" is a fixed guess; rows with a wrapped title, multiple badges, warning/error text, and now the added GapNote line can exceed 80px, which can cause a visible jump when an off-screen content-visibility: auto row scrolls into view and its real height replaces the placeholder. Cosmetic only.

Filed from a Claude Code session

Non-blocking findings from the auto code review of **PR #670** (perf(import): review queue keys ~90ms → ~3ms; flag big price gaps (Check price)) — captured for batch cleanup rather than blocking the PR. Commit `31479db` · https://github.com/Gambit-Inc/gambit/pull/670 SHOULD-FIX: - `frontend/app/hq/imports/page.js:1705-1707` — `ParkedPanel` now owns its own local `picks` state instead of sharing the wizard-level `picks` passed down from `ImportWizard`/`TriageStep`. Previously the same `picks` object (keyed by `row_id`) was shared across the queue and the Parked panel, so a candidate index chosen while a row was still in the queue carried over once the row was set aside. Now that selection silently resets to index 0 the moment a row lands in Parked, because the new local `useState({})` starts empty. Not data-destructive (nothing commits until the seller picks again), but it's an unannounced UX regression from the pre-refactor behavior. NITS: - `frontend/app/hq/imports/page.js:1784` vs `:1818` — the new top-level `const setPick = useCallback((id, i) => ...)` and the pre-existing keyboard handler's local `const setPick = (i) => ...` inside `onKey` have the same name but different signatures (row-id-based vs index-based). The inner one shadows the outer; functionally harmless but confusing to read/lint. - `frontend/app/hq/imports/_components/QueueRow.jsx:33` — `containIntrinsicSize: "auto 80px"` is a fixed guess; rows with a wrapped title, multiple badges, warning/error text, and now the added `GapNote` line can exceed 80px, which can cause a visible jump when an off-screen `content-visibility: auto` row scrolls into view and its real height replaces the placeholder. Cosmetic only. --- Filed from a Claude Code session

gambit-admin added the enhancement label 2026-09-30 20:20:04 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gambit/gambit#903