feat: add Canpar, Purolator, and Spee-Dee courier definitions#116
feat: add Canpar, Purolator, and Spee-Dee courier definitions#116cbillen wants to merge 3 commits intojkeen:mainfrom
Conversation
Adds three new Canadian/regional courier definitions, each verified against real tracking numbers: - Canpar (22): letter [CDKLSUXZ] + 21 digits; no mathematical checksum (the 22nd character is a piece sequence number, not a check digit); regex-only validation. - Purolator (12): 12-digit PIN with verified Luhn checksum (17/17 real numbers confirmed); first digit constrained to [0-5] to avoid false positives against FedEx Express 12-digit numbers. - Purolator (alpha + 9): 3 uppercase letters + 9 digits; regex-only. - Spee-Dee Delivery (20): SP prefix + exactly 18 digits; no mathematical checksum (confirmed against 10 real numbers); regex-only validation. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds three new courier configuration JSON files (Canpar, Purolator, Spee-Dee) defining tracking number formats, optional validation rules, tracking URL templates, and test sample numbers. No code or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
couriers/canpar.json (1)
12-22: All valid test numbers use only theDprefix — consider adding coverage for the other 7 prefix letters.The regex allows
[CDKLSUXZ](8 variants), but every valid sample starts withD. A test number for at least one other prefix (e.g.,C,K,L,S,U,X,Z) would guard against silent regressions if the character class is narrowed or reordered in a future edit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@couriers/canpar.json` around lines 12 - 22, The test data for Canpar tracking numbers only uses the "D" prefix in the "valid" array, but the validator accepts any of [CDKLSUXZ]; add at least one additional valid example (e.g., a tracking string starting with "C" or "K") to the "valid" list in couriers/canpar.json so tests exercise another prefix and prevent future regressions in the character class used by the regex.couriers/purolator.json (1)
8-8: Trailing\s*is captured insideCheckDigitrather than outside — inconsistent with the other patterns.Every other regex in this PR closes all named groups before the final
\s*. Here the whitespace is swallowed intoCheckDigit:(?<CheckDigit>[0-9]\s*) ← whitespace is part of the captured valueWhile the library likely strips whitespace before the Luhn computation (tests pass), the captured
CheckDigitvalue will include any trailing spaces, unlike every other pattern.♻️ Proposed fix — move trailing `\s*` outside the group
-"regex": "\\s*(?<SerialNumber>[0-5]\\s*([0-9]\\s*){10})(?<CheckDigit>[0-9]\\s*)", +"regex": "\\s*(?<SerialNumber>[0-5]\\s*([0-9]\\s*){10})(?<CheckDigit>[0-9])\\s*",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@couriers/purolator.json` at line 8, The regex currently captures trailing whitespace inside the named group "CheckDigit" (regex property in couriers/purolator.json) — move the final \s* so it sits after the closing parenthesis of the (?<CheckDigit>...) group (i.e., change (?<CheckDigit>[0-9]\s*) to (?<CheckDigit>[0-9])\s*) so the CheckDigit capture contains only the digit, consistent with the other patterns that close all named groups before the trailing whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@couriers/purolator.json`:
- Around line 33-46: The regex field currently uses a broad pattern
"\\s*(?<SerialNumber>([A-Z]\\s*){3}([0-9]\\s*){9})\\s*" which will match any 3
uppercase letters + 9 digits and cause false positives; tighten it by replacing
the ([A-Z]{3}) portion with an alternation of known Purolator prefixes (e.g.,
(?:KYV|CGK|JFV|TLR|...)) or otherwise restrict the allowed prefix set, or if
that’s not possible add a clear comment in the JSON explaining why a broad class
is required and add a secondary validation rule in the "validation" object
(e.g., prefix whitelist or checksum logic) to reduce collisions so consumers use
both the refined regex and the "validation" checks to accept numbers.
---
Nitpick comments:
In `@couriers/canpar.json`:
- Around line 12-22: The test data for Canpar tracking numbers only uses the "D"
prefix in the "valid" array, but the validator accepts any of [CDKLSUXZ]; add at
least one additional valid example (e.g., a tracking string starting with "C" or
"K") to the "valid" list in couriers/canpar.json so tests exercise another
prefix and prevent future regressions in the character class used by the regex.
In `@couriers/purolator.json`:
- Line 8: The regex currently captures trailing whitespace inside the named
group "CheckDigit" (regex property in couriers/purolator.json) — move the final
\s* so it sits after the closing parenthesis of the (?<CheckDigit>...) group
(i.e., change (?<CheckDigit>[0-9]\s*) to (?<CheckDigit>[0-9])\s*) so the
CheckDigit capture contains only the digit, consistent with the other patterns
that close all named groups before the trailing whitespace.
| "regex": "\\s*(?<SerialNumber>([A-Z]\\s*){3}([0-9]\\s*){9})\\s*", | ||
| "validation": {}, | ||
| "tracking_url": "https://www.purolator.com/en/shipping/tracker?searchValue=%s", | ||
| "test_numbers": { | ||
| "valid": [ | ||
| "KYV009956937", | ||
| "CGK002986959", | ||
| "JFV247545960", | ||
| "TLR000083964" | ||
| ], | ||
| "invalid": [ | ||
| "KY009956937", | ||
| "331426749957" | ||
| ] |
There was a problem hiding this comment.
[A-Z]{3}[0-9]{9} is a very broad pattern — high false-positive risk.
Any string of 3 uppercase letters followed by 9 digits will match. There are no carrier-specific prefix constraints to narrow this down. With no checksum to act as a secondary gate, this definition could spuriously match tracking numbers from other carriers that also use alphanumeric prefixes.
Consider whether known Purolator three-letter prefixes can be enumerated (e.g., (?:KYV|CGK|JFV|TLR|...)) to reduce the collision surface, or at minimum note in a comment why the open-ended character class is necessary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@couriers/purolator.json` around lines 33 - 46, The regex field currently uses
a broad pattern "\\s*(?<SerialNumber>([A-Z]\\s*){3}([0-9]\\s*){9})\\s*" which
will match any 3 uppercase letters + 9 digits and cause false positives; tighten
it by replacing the ([A-Z]{3}) portion with an alternation of known Purolator
prefixes (e.g., (?:KYV|CGK|JFV|TLR|...)) or otherwise restrict the allowed
prefix set, or if that’s not possible add a clear comment in the JSON explaining
why a broad class is required and add a secondary validation rule in the
"validation" object (e.g., prefix whitelist or checksum logic) to reduce
collisions so consumers use both the refined regex and the "validation" checks
to accept numbers.
- canpar: add K-prefix valid test number to exercise the full [CDKLSUXZ] character class, not just D - purolator (12): move trailing \s* outside the CheckDigit group so the captured value contains only the digit, consistent with every other pattern in the repo Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks for the review — two of the three items are fixed in the follow-up commit: Fixed:
Re: The open character class is intentional — Purolator's 3-letter prefix set is not publicly documented anywhere I could find. The known examples span at least 9 distinct prefixes (KYV, CGK, JFV, TLR from the test numbers; ANR, DCM, CGJ, YAW from TrackingMore; KTE from k2track) with no apparent structural rule governing the letters, so an enumerated alternation would silently reject valid Purolator numbers that use unlisted prefixes. Happy to tighten it further if you know of an authoritative prefix list, or to accept the known trade-off and leave a note in the PR for future editors. |
Replaces the synthetically-constructed K576002... test number added during review with real L and S service-letter tracking numbers, confirming three distinct prefixes from actual shipment data. Also includes a piece-10 example (D576002440000001718010) and a piece-4 example (D576002440000001428004) which demonstrate that the 3-digit trailing field is a piece sequence number (001–NNN), not a check digit — a non-sequential multi-piece shipment (pieces 002 and 004 with no 003) was observed in the same dataset. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Follow-up: upgraded the Canpar test numbers with 20 real tracking numbers spanning three service letter prefixes (D, L, S). The new data also gives a cleaner picture of the trailing 3-digit field — it is a piece sequence number, not a check digit:
The synthetic |
Summary
Adds three new courier definitions, each validated against real tracking numbers:
Canpar —
[CDKLSUXZ]+ 21 digits (22 chars total). No mathematical checksum: the 22nd character is a piece sequence number (001, 002, 003…) for multi-piece shipments, not a computed check digit. Verified against 10 real tracking numbers.Purolator (12-digit PIN) — Luhn checksum confirmed against 17 real tracking numbers (7 from public sources + 10 directly provided). First digit constrained to
[0-5]to prevent false-positive collisions with FedEx Express 12-digit numbers, whose test variants all start with 4, 7, or 9.Purolator (alpha + 9) — 3 uppercase letters + 9 digits (e.g.
KYV009956937). Regex-only; no checksum data available.Spee-Dee Delivery —
SPprefix + exactly 18 digits (20 chars total). No mathematical checksum confirmed against 10 real tracking numbers; validated by prefix and length per vendor documentation.Verification
All definitions pass the full test suite (
bundle exec rake, 2690 examples, 0 failures).The Luhn checksum for Purolator is already supported via
validates_luhn?inspec/spec_helper.rb.Notable finding during development: the Canpar 22nd-character was initially suspected to be a mod10 check digit per various third-party integration docs, but real tracking number analysis showed multiple numbers sharing identical first-21 characters with only the last digit differing (1, 2, 3) — conclusively identifying it as a piece sequence number.
Made with Cursor
Summary by CodeRabbit