feat: add parser for axis card

[npv12]

Important

Adds AxisParser for Axis bank CSV parsing and tests for robust error handling and transaction parsing.

• New Parser:
  • Adds AxisParser in axis_parser.go for parsing Axis bank CSV statements.
  • Implements parseTransactionRow() to handle transaction row parsing with error handling for invalid data.
  • Uses regex patterns for transaction description parsing.
• Tests:
  • Adds axis_parser_test.go for testing AxisParser with various scenarios, including valid/invalid data and edge cases.
  • Adds axis_credit_parser_test.go for testing Axis credit card statement parsing, including handling of different amount formats and header variations.
• Error Handling:
  • Handles errors for missing headers, invalid dates, and malformed amounts in both CSV and XLSX parsers.
  • Truncates long transaction descriptions to 40 characters in transaction names.

^{This description was created by for b009609. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to c40ad89 in 1 minute and 57 seconds. Click for details.

• Reviewed 278 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/parser/axis_parser.go:97

• Draft comment:
  Handle the ambiguous case when both debit and credit fields are non-empty explicitly.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is asking to "handle the ambiguous case when both debit and credit fields are non-empty explicitly." Looking at the code, it uses if-else-if logic, so when both are non-empty, debitStr takes precedence. The comment is essentially suggesting that this case should be handled differently - perhaps with an error or explicit logging. However, this is speculative - the comment doesn't provide evidence that having both fields non-empty is actually problematic or that the current behavior is wrong. It's asking the author to consider a case that may or may not be an issue in practice. This violates the rule about not making speculative comments ("If X, then Y is an issue"). The comment doesn't demonstrate that this is definitely a problem with real Axis bank CSV data. Maybe the comment is valid because having both debit and credit non-empty in a single transaction row could indicate malformed data, and silently choosing one over the other could hide data quality issues. Perhaps explicit handling would be better for debugging. While that's a reasonable concern, the comment doesn't provide evidence that this actually happens with Axis bank CSVs or that it's definitely a problem. The current behavior (debit takes precedence) is a valid design choice. Without evidence that this causes issues, this is a speculative suggestion about a potential edge case, which violates the rules. This comment is speculative - it suggests handling an edge case (both fields non-empty) without evidence that it's actually problematic. The current code has a clear behavior (debit takes precedence), which is a valid design choice. Per the rules against speculative comments, this should be deleted.

2. server/internal/parser/axis_parser_test.go:24

• Draft comment:
  Add tests for rows where both debit and credit values are provided to confirm intended behavior.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is suggesting adding a test case for a scenario that isn't currently tested. This is a test suggestion, not a bug fix or required change. The rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make speculative comments." This comment is asking the author to add more test coverage for an edge case (both DR and CR populated). While this could be good practice, it's not clearly a required code change - it's more of a suggestion to improve test coverage. The comment doesn't indicate that there's a bug or that the current implementation is wrong. It's asking to "confirm intended behavior" which is explicitly mentioned in the rules as something to avoid. This could be a legitimate edge case that needs handling - if the parser encounters a row with both debit and credit values, it might behave unexpectedly or incorrectly. The comment might be highlighting a real gap in test coverage that could catch bugs. While it could be a legitimate edge case, the comment doesn't provide evidence that this is actually a problem or that the current implementation handles it incorrectly. It's asking to "confirm intended behavior" which is explicitly against the rules. Without seeing the actual implementation or evidence that this causes issues, this is speculative test coverage improvement rather than a required fix. This comment is asking for additional test coverage to confirm behavior, which violates the rule against asking authors to confirm or verify things. It's not pointing out a clear bug or required change, just suggesting more comprehensive testing. This should be deleted.

3. server/internal/parser/axis_parser_test.go:40

• Draft comment:
  Typo detected: 'Accoun' in the string "UPI/P2M/509038927105/Yes Bank Partner Sell/Accoun/YesBank_Yespay" seems to be missing a 't' (should be 'Account').
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test file with synthetic test data. The string in question appears to be mock transaction data used to test the parser. While "Accoun" might be a typo, there are several considerations: 1) This is test data, not production code, 2) The test is checking that the parser can handle the data and extract "UPI to" from the Name field - the typo doesn't affect the test's validity, 3) Real-world bank data might actually contain typos or truncated text, so testing with imperfect data could be intentional, 4) The comment is about a minor spelling issue in test data that doesn't affect functionality. According to the rules, I should not comment on things that are obvious or unimportant, and this seems to fall into that category. Could this be intentional test data that mimics real-world imperfect data? The parser might need to handle malformed or truncated strings, so having "Accoun" instead of "Account" could be testing robustness rather than being a mistake. Even if it's unintentional, this is a trivial typo in test data that doesn't affect the test's purpose or functionality. The test is verifying that the parser can extract transaction information, and whether the mock data says "Accoun" or "Account" is irrelevant to that goal. This falls under "obvious or unimportant" comments that should be removed. This comment should be deleted. It's pointing out a minor typo in synthetic test data that has no impact on the test's functionality or purpose. This is an unimportant observation that doesn't require a code change.

Workflow ID: wflow_1PpE3G4KVaZJyvNN

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed 9d2f721 in 1 minute and 50 seconds. Click for details.

• Reviewed 70 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/parser/axis_parser.go:30

• Draft comment:
  The last regex pattern uses '(?i)INT.PD|Int.Pd'. Since (?i) applies case-insensitively, the '|Int.Pd' alternative is redundant. Consider simplifying this pattern.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is technically correct - the regex pattern does have redundancy. However, I need to consider: 1) This is a minor code quality issue, 2) The pattern itself wasn't changed in this PR - it was just moved from a local variable to a package-level variable, 3) The redundancy doesn't affect functionality or performance, 4) This is the kind of minor optimization that could be considered nitpicky. The main change in this PR was the performance optimization of precompiling the regex patterns, not fixing the patterns themselves. The comment is about code that existed before and wasn't modified in substance, just relocated. While the pattern was moved in this diff, one could argue that since the code is being touched anyway, it's a good opportunity to clean up the redundancy. The comment is actionable and clear, suggesting a specific improvement. It's not purely informative - it suggests a concrete code change. Although the code was touched, the actual regex pattern string itself wasn't modified - it was just moved. The PR's purpose was performance optimization through precompilation, not fixing existing regex patterns. This comment is about pre-existing code quality rather than issues introduced or directly addressed by the changes. According to the rules, comments should be about changes made, and this is more of a general code quality suggestion on code that happened to be moved. This comment should be deleted. While technically correct, it's a minor code quality issue on a regex pattern that wasn't actually modified (just relocated). The comment is not about the substantive changes in the PR (performance optimization through precompilation), and represents the kind of minor nitpick that doesn't require action.

2. server/internal/parser/axis_parser.go:147

• Draft comment:
  When a pattern matches but has no capture groups (e.g., Interest), the function always returns pattern.creditName regardless of the transaction type. Ensure this is intentional or consider using isCredit to choose between creditName and debitName.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_7DMGrORkaNadwMF0

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 60cdb98 in 28 seconds. Click for details.

• Reviewed 21 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/parser/axis_parser_test.go:3

• Draft comment:
  Removal of TestAxisParser is OK if a separate Ginkgo bootstrap (e.g., axis_parser_suite_test.go) exists to register the specs.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 1% The comment is asking the author to ensure that a separate Ginkgo bootstrap exists, which violates the rule against asking the author to confirm or ensure something. It is not making a specific code suggestion or pointing out a specific issue.

Workflow ID: wflow_8tlLnYls78rkemPy

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[codecov]

Codecov Report

❌ Patch coverage is 91.30435% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
server/internal/parser/axis_parser.go	91.30%	5 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 648c8a0 in 20 seconds. Click for details.

• Reviewed 25 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/parser/axis_parser.go:155

• Draft comment:
  Remove trailing whitespace for cleaner formatting.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

2. server/internal/parser/axis_parser_test.go:9

• Draft comment:
  Remove extraneous blank line after import block.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

Workflow ID: wflow_243B7cVAb1S0dIVA

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed b009609 in 2 minutes and 3 seconds. Click for details.

• Reviewed 134 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 17 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. server/internal/parser/axis_credit_parser_test.go:139

• Draft comment:
  Good edge case for parseAmount using only '₹'; ensure the error message remains 'empty amount after cleaning'.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

2. server/internal/parser/axis_credit_parser_test.go:145

• Draft comment:
  Truncation test for long description is clear; consider verifying behavior with multi-byte characters.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

3. server/internal/parser/axis_credit_parser_test.go:155

• Draft comment:
  Error test for missing amount string is appropriate; the 'amount not found' substring check is explicit.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

4. server/internal/parser/axis_credit_parser_test.go:163

• Draft comment:
  Non-transaction row test (invalid date) returns nil as expected.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

5. server/internal/parser/axis_credit_parser_test.go:170

• Draft comment:
  Test for header present but no valid transactions robustly verifies error handling.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

6. server/internal/parser/axis_parser_test.go:19

• Draft comment:
  Insufficient columns error test is clear and helps prevent index errors.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

7. server/internal/parser/axis_parser_test.go:26

• Draft comment:
  Invalid date test is concise and ensures date parsing failures are caught.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

8. server/internal/parser/axis_parser_test.go:32

• Draft comment:
  Debit row parsing test properly checks for 'UPI to' substring and correct amount conversion.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

9. server/internal/parser/axis_parser_test.go:42

• Draft comment:
  Credit row test correctly expects a negative amount with appropriate descriptor ('IMPS from').
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

10. server/internal/parser/axis_parser_test.go:53

• Draft comment:
  Synthetic CSV test ensures header recognition and filters potential PII; verify if 'SAMPLE USER' needs masking.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

11. server/internal/parser/axis_parser_test.go:87

• Draft comment:
  Test for unrecognized CSV header is robust; error expectation is clear.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

12. server/internal/parser/axis_parser_test.go:96

• Draft comment:
  Test skipping rows with fewer than 6 columns prevents accidental parsing of malformed rows.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

13. server/internal/parser/axis_parser_test.go:105

• Draft comment:
  Long description truncation for AxisParser enforces a 'Debit: ' prefix and 40 rune length; check consistency with AxisCreditParser if intentional.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to "check consistency" with another parser (AxisCreditParser), which suggests this is a speculative concern rather than a definite issue. The test itself is testing the actual behavior of the AxisParser, so the test is documenting what the parser does. The comment doesn't point to a clear bug or issue - it's asking the author to verify consistency across files. This violates the rule about not making speculative comments and requiring cross-file context. The comment also doesn't suggest a specific code change - it's asking the author to check something, which is not actionable. The comment might be valid if there's an actual inconsistency between parsers that would cause user-facing issues. Perhaps the truncation behavior should be consistent across different parser types for the same bank. Even if there is an inconsistency, the comment doesn't provide evidence of it - it just asks the author to check. Without seeing the AxisCreditParser implementation, we can't determine if this is a real issue. The rules explicitly state to ignore cross-file issues and only focus on the file being reviewed. This comment requires context from another file to be actionable. This comment should be deleted because it's speculative, requires cross-file context to evaluate, and asks the author to verify/check something rather than pointing to a clear issue. It violates the rule about ignoring cross-file issues and not making speculative comments.

14. server/internal/parser/axis_parser_test.go:117

• Draft comment:
  NEFT and RTGS pattern parsing test is effective in ensuring proper transaction name formatting.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

15. server/internal/parser/axis_parser_test.go:129

• Draft comment:
  Test enforcing an error on empty transaction date ensures data integrity.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

16. server/internal/parser/axis_parser_test.go:137

• Draft comment:
  Non-float credit amount error test is clear; error message check for 'failed to parse credit amount' is explicit.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

17. server/internal/parser/axis_parser_test.go:145

• Draft comment:
  Test for both empty debit and credit amounts is comprehensive; error message verification maintains robustness.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 1% None

Workflow ID: wflow_eHgC61AhFbZ0SgwQ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}