Implement card BIN functionality#29
Merged
Merged
Conversation
WalkthroughThis pull request extends bank name lookup to accept CLABE (18 digits) or card numbers (15–16 digits). It adds a CARD_BINS lookup table, updates TypeScript declarations, implements getBankName routing (ABM for CLABE, BIN for cards), updates getBankNameOrThrow, expands tests for card/BIN paths and error cases, and bumps the package version. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/test.js`:
- Line 21: The test constant VALID_CARD_BBVA is currently a Luhn-valid PAN
('4152314501669421'); change its value to a clearly synthetic PAN that preserves
the BIN prefix 415231 but cannot pass Luhn (e.g., replace the trailing digits
with zeros) so tests still validate BIN logic but do not use a real-looking card
number — update the constant named VALID_CARD_BBVA accordingly in test/test.js.
- Line 5: Remove the unused CARD_BINS import from the destructured import list
in test/test.js (CARD_BINS is causing ESLint no-unused-vars); locate the import
statement that includes CARD_BINS and delete that identifier, or alternatively
add a minimal smoke test that references CARD_BINS (e.g., asserting it is
defined) so the symbol is used — update the import or add the test accordingly
to satisfy the linter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 20f3bd2f-383d-4ad5-ae5f-225665a646fb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
lib/card_bins.jslib/index.d.tslib/index.jstest/test.js
julietteceb16
force-pushed
the
feature/add-card-bins-lookup
branch
from
09b537f to
07db5cb
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/test.js (1)
20-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
VALID_CARD_BBVAis still a Luhn-valid PAN — replace the suffix with zeros.
'4152314501669421'passes the Luhn check and is flagged with confidence 1.0 by two independent analyzers (OpenGrep + Microsoft Presidio). Since the BIN prefix415231is the only meaningful part for these tests, replace the account-specific digits with zeros to make it unambiguously synthetic.🐛 Proposed fix
-const VALID_CARD_BBVA = '4152314501669421'; // BIN 415231 -> 40012 -> BBVA Mexico +const VALID_CARD_BBVA = '4152310000000000'; // BIN 415231 -> 40012 -> BBVA Mexico🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/test.js` at line 20, Replace the Luhn-valid PAN string in VALID_CARD_BBVA so the test uses a synthetic, non-Luhn account portion: update the constant VALID_CARD_BBVA (currently '4152314501669421') to keep the BIN prefix '415231' and set the remaining account digits to zeros (e.g., '4152310000000000' or similar length-preserving zero suffix) so analyzers no longer flag it as a real card number.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@test/test.js`:
- Line 20: Replace the Luhn-valid PAN string in VALID_CARD_BBVA so the test uses
a synthetic, non-Luhn account portion: update the constant VALID_CARD_BBVA
(currently '4152314501669421') to keep the BIN prefix '415231' and set the
remaining account digits to zeros (e.g., '4152310000000000' or similar
length-preserving zero suffix) so analyzers no longer flag it as a real card
number.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a1667a73-68d5-4a2f-a262-d1d5101ff0d8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
lib/card_bins.jslib/index.d.tslib/index.jstest/test.js
✅ Files skipped from review due to trivial changes (1)
- lib/card_bins.js
gmorales96
reviewed
May 11, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/index.js`:
- Around line 49-53: The current checks only verify CLABE length and prefix when
resolving bank names; update both lookup sites (the block using
account.substring(0, 3) and the similar branch around lines 74-81) to validate
the full 18-digit CLABE using the official CLABE control-digit algorithm before
performing the ABM/bank-name lookup. Implement or call a helper like
validateClabe(account) that computes and verifies the control digit (using the
standard weights and modulus 10 rule) and only proceed to derive code =
account.substring(0,3) and BANK_NAMES[BANKS[code]] if validateClabe returns
true; otherwise return null (or the existing invalid-input response) to preserve
the contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c68135a0-f187-465a-9616-658b209a3aab
📒 Files selected for processing (4)
lib/index.d.tslib/index.jspackage.jsontest/test.js
✅ Files skipped from review due to trivial changes (1)
- package.json
Comment on lines
+49
to
+53
| if (account.length === CLABE_LENGTH) { | ||
| const code = account.substring(0, 3); | ||
| const bankName = BANK_NAMES[BANKS[code]]; | ||
| return bankName === undefined ? null : bankName; | ||
| } |
There was a problem hiding this comment.
Validate full CLABE before ABM lookup in both APIs.
On Line 49 and Line 74, the 18-char path only checks length and prefix, so invalid CLABEs (including wrong control digit) can still resolve to a bank name. That breaks the documented invalid-input contract.
🐛 Proposed fix
function getBankName(account) {
@@
if (account.length === CLABE_LENGTH) {
+ if (!validateClabe(account)) return null;
const code = account.substring(0, 3);
const bankName = BANK_NAMES[BANKS[code]];
return bankName === undefined ? null : bankName;
}
if (account.length >= 15 && account.length <= 16) {
+ if (!isANumber(account)) return null;
const bin = account.substring(0, 6);
const bankName = BANK_NAMES[CARD_BINS[bin]];
return bankName === undefined ? null : bankName;
}
@@
function getBankNameOrThrow(account) {
@@
if (account.length === CLABE_LENGTH) {
+ if (!validateClabe(account)) {
+ throw new Error('Cuenta inválida: CLABE no válida');
+ }
const code = account.substring(0, 3);
const bankName = BANK_NAMES[BANKS[code]];
@@
if (account.length >= 15 && account.length <= 16) {
+ if (!isANumber(account)) {
+ throw new Error('Cuenta inválida: tarjeta debe contener solo dígitos');
+ }
const bin = account.substring(0, 6);
const bankName = BANK_NAMES[CARD_BINS[bin]];Also applies to: 74-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/index.js` around lines 49 - 53, The current checks only verify CLABE
length and prefix when resolving bank names; update both lookup sites (the block
using account.substring(0, 3) and the similar branch around lines 74-81) to
validate the full 18-digit CLABE using the official CLABE control-digit
algorithm before performing the ABM/bank-name lookup. Implement or call a helper
like validateClabe(account) that computes and verifies the control digit (using
the standard weights and modulus 10 rule) and only proceed to derive code =
account.substring(0,3) and BANK_NAMES[BANKS[code]] if validateClabe returns
true; otherwise return null (or the existing invalid-input response) to preserve
the contract.
gmorales96
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Chores