fix: address code review feedback on PR #38

normalizeCodeStatus: replace parseInt with /^\d+$/ guard so partial
numeric strings like '4foo' are no longer misclassified as a known
status code.

deleteUserRedeemedCodes: replace RETURNING-all-ids approach with a
COUNT(*) + DELETE pair to avoid materialising a large array of row IDs
for users with many redemption records.

backfillHandler: validate instance_id is present and non-'0' after
confirming playerData.details exists, matching the check in
autoRedeemer.ts. Skip the submitCode call when instance_id is invalid.

backfillHandler: replace the N+1 isCodeRedeemed/addPendingCode loop
with a single getRedeemedCodesFromList query to build the excluded set,
then bulk-insert the remainder via addNewPendingCodes. Adds
getRedeemedCodesFromList to codeManager (SELECT … IN with Set return).

Tests: add normalizeCodeStatus partial-string case, getRedeemedCodesFromList
unit tests, and a backfillHandler scenario covering invalid instance_id.

Signed-off-by: Michael Cramer <michael@bigmichi1.de>

Author: BigMichi1

src/bot/database/codeManager.test.ts

@@ -59,6 +59,10 @@ describe('normalizeCodeStatus', () => {
   test('normalizes numeric string "4" to Code Expired', () => {
     expect(normalizeCodeStatus('4')).toBe('Code Expired');
   });
+  test('treats partial-numeric strings as canonical (not parsed as int)', () => {
+    expect(normalizeCodeStatus('4foo')).toBe('4foo');
+    expect(normalizeCodeStatus('0bar')).toBe('0bar');
+  });
 });
 
 // ---------------------------------------------------------------------------
@@ -447,6 +451,36 @@ describe('deleteUserRedeemedCodes', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// getRedeemedCodesFromList
+// ---------------------------------------------------------------------------
+describe('getRedeemedCodesFromList', () => {
+  test('returns empty set for empty codes list', async () => {
+    const result = await codeManager.getRedeemedCodesFromList([]);
+    expect(result.size).toBe(0);
+  });
+  test('returns empty set when no codes have been redeemed', async () => {
+    const result = await codeManager.getRedeemedCodesFromList(['CODE1234ABCD']);
+    expect(result.size).toBe(0);
+  });
+  test('returns codes with Success status', async () => {
+    await codeManager.addRedeemedCode('CODE1234ABCD', USER_A, 'Success');
+    const result = await codeManager.getRedeemedCodesFromList(['CODE1234ABCD', 'CODE1111AAAA']);
+    expect(result.has('CODE1234ABCD')).toBe(true);
+    expect(result.has('CODE1111AAAA')).toBe(false);
+  });
+  test('returns codes with Code Expired status', async () => {
+    await codeManager.addRedeemedCode('CODE1234ABCD', USER_A, 'Code Expired');
+    const result = await codeManager.getRedeemedCodesFromList(['CODE1234ABCD']);
+    expect(result.has('CODE1234ABCD')).toBe(true);
+  });
+  test('excludes non-qualifying statuses like Invalid Parameters', async () => {
+    await codeManager.addRedeemedCode('CODE1234ABCD', USER_A, 'Invalid Parameters');
+    const result = await codeManager.getRedeemedCodesFromList(['CODE1234ABCD']);
+    expect(result.has('CODE1234ABCD')).toBe(false);
+  });
+});
+
 // ---------------------------------------------------------------------------
 // getRedeemedCodesByUsers
 // ---------------------------------------------------------------------------

src/bot/database/codeManager.ts

@@ -18,11 +18,13 @@ const CODE_STATUS_MAP: Record<number, string> = {
  * to the canonical status string stored in the database.
  */
 export function normalizeCodeStatus(status: number | string): string {
-  const n = typeof status === 'number' ? status : parseInt(String(status), 10);
-  if (!isNaN(n)) {
-    return CODE_STATUS_MAP[n] ?? 'Unknown Status';
+  if (typeof status === 'number') {
+    return CODE_STATUS_MAP[status] ?? 'Unknown Status';
   }
-  return String(status);
+  if (/^\d+$/.test(status)) {
+    return CODE_STATUS_MAP[Number(status)] ?? 'Unknown Status';
+  }
+  return status;
 }
 
 export const CHEST_TYPE_NAMES: Record<number, string> = {
@@ -292,13 +294,34 @@ class CodeManager {
     return inserted.map((r) => r.code);
   }
 
+  /**
+   * Returns the subset of `codes` that already have at least one
+   * Success or Code Expired row in redeemed_codes (i.e. globally redeemed).
+   * Uses a single IN query — suitable for bulk pre-filter before addNewPendingCodes.
+   */
+  async getRedeemedCodesFromList(codes: string[]): Promise<Set<string>> {
+    if (codes.length === 0) return new Set();
+    const rows = db
+      .select({ code: redeemedCodes.code })
+      .from(redeemedCodes)
+      .where(
+        and(
+          inArray(redeemedCodes.code, codes),
+          or(eq(redeemedCodes.status, 'Success'), eq(redeemedCodes.status, 'Code Expired'))
+        )
+      )
+      .all();
+    return new Set(rows.map((r) => r.code));
+  }
+
   async deleteUserRedeemedCodes(discordId: string): Promise<number> {
-    const deleted = db
-      .delete(redeemedCodes)
+    const countRow = db
+      .select({ count: sql<number>`count(*)` })
+      .from(redeemedCodes)
       .where(eq(redeemedCodes.discordId, discordId))
-      .returning({ id: redeemedCodes.id })
-      .all();
-    return deleted.length;
+      .get();
+    db.delete(redeemedCodes).where(eq(redeemedCodes.discordId, discordId)).run();
+    return countRow?.count ?? 0;
   }
 
   /**

src/bot/handlers/backfillHandler.test.ts

@@ -278,3 +278,31 @@ describe('Scenario 3: duplicate backfill runs do not create duplicate pending co
     expect(hits).toHaveLength(1);
   });
 });
+
+// ---------------------------------------------------------------------------
+// Scenario 4 — invalid instance_id skips submission but stores codes as pending
+// ---------------------------------------------------------------------------
+
+describe('Scenario 4: invalid instance_id skips code submission', () => {
+  test('codes become pending when instance_id is missing from user details', async () => {
+    await userManager.saveCredentials({
+      discordId: USER,
+      userId: '316463',
+      userHash: 'f4e6d3dbc34173d23e7d198e4a8fc773',
+      server: MOCK_SERVER,
+    });
+
+    // details object exists but instance_id is absent (falsy → treated as '0')
+    getUserDetailsSpy.mockResolvedValue({ details: {} } as any);
+
+    const stats = await backfillChannelHistory(makeChannel([CODE_A, CODE_B]));
+
+    expect(stats.codesRedeemed).toBe(0);
+    expect(submitCodeSpy).not.toHaveBeenCalled();
+
+    const pending = await codeManager.getPendingCodes();
+    expect(pending).toContain(CODE_A);
+    expect(pending).toContain(CODE_B);
+  });
+});
+

src/bot/handlers/backfillHandler.ts

@@ -192,7 +192,13 @@ export async function backfillChannelHistory(
               continue;
             }
 
-            const instanceId = playerData.details?.instance_id || '';
+            const instanceId = playerData.details?.instance_id || '0';
+            if (!instanceId || instanceId === '0') {
+              logger.warn(
+                `[BACKFILL] Skipping code ${code} for user ${user.discordId}: invalid instance_id`
+              );
+              continue;
+            }
 
             const response = await IdleChampionsApi.submitCode({
               server,
@@ -234,15 +240,12 @@ export async function backfillChannelHistory(
     }
 
     // Store any remaining codes as pending so /catchup can find them later.
-    // addPendingCode is idempotent (onConflictDoNothing) so running backfill
-    // multiple times is safe.
-    for (const code of allCodes) {
-      const isRedeemed = await codeManager.isCodeRedeemed(code);
-      if (!isRedeemed) {
-        await codeManager.addPendingCode(code);
-        stats.pendingCodes++;
-      }
-    }
+    // Single query to find already-redeemed codes, then bulk-insert the rest.
+    const allCodesArr = [...allCodes];
+    const redeemedSet = await codeManager.getRedeemedCodesFromList(allCodesArr);
+    const codesToPend = allCodesArr.filter((c) => !redeemedSet.has(c));
+    const inserted = await codeManager.addNewPendingCodes(codesToPend);
+    stats.pendingCodes = inserted.length;
 
     onProgress?.(
       `✅ Backfill complete! Found: ${stats.codesFound}, Redeemed: ${stats.codesRedeemed}, Pending: ${stats.pendingCodes}`