feat(Bank Reconcilitation Beta): prioritize ranking parameters#331
feat(Bank Reconcilitation Beta): prioritize ranking parameters#331PatrickDEissler wants to merge 5 commits intoversion-15-hotfixfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces weighted ranking parameters for the Bank Reconciliation Beta feature to prioritize matching criteria. The change addresses the issue where all parameters (date, name, amount, reference) had equal weight in voucher matching, which was suboptimal for real-world scenarios where exact amounts are more reliable indicators than posting dates (especially for Purchase Invoices).
Changes:
- Added six weight constants (REF_RANK_WEIGHT=3, PARTY_RANK_WEIGHT=2, AMOUNT_RANK_WEIGHT=2, DATE_RANK_WEIGHT=1, NAME_MATCH_WEIGHT=3, REF_MATCH_WEIGHT=3) to prioritize ranking parameters
- Updated all rank_expression calculations across 10 matching query functions to apply these weights
- Removed the post-processing rank increment for reference number matches found in transaction descriptions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...rna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py
Show resolved
Hide resolved
...rna_kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| reference_no = voucher["reference_no"] | ||
| if reference_no and (reference_no.strip() in transaction.description): | ||
| voucher["rank"] += 1 | ||
| voucher["name_in_desc_match"] = 1 |
There was a problem hiding this comment.
check_matching sets name_in_desc_match when the voucher’s reference_no is found in the bank transaction description. This conflicts with the established meaning of name_in_desc_match (it’s used by the Match tab to bold the voucher name link), and it also means the reference-column highlighting (ref_in_desc_match) won’t be set for non-invoice doctypes. Consider either (a) matching against voucher["name"] when setting name_in_desc_match, or (b) setting a dedicated ref_in_desc_match flag (and keeping name_in_desc_match for actual name matches).
| voucher["name_in_desc_match"] = 1 | |
| voucher["ref_in_desc_match"] = 1 |
| rank_expression = ( | ||
| ref_rank + party_rank + currency_match + amount_rank + date_rank + name_match + ref_match + 1 | ||
| (ref_rank * REF_RANK_WEIGHT) | ||
| + (party_rank * PARTY_RANK_WEIGHT) | ||
| + (amount_rank * AMOUNT_RANK_WEIGHT) | ||
| + (date_rank * DATE_RANK_WEIGHT) | ||
| + (name_match * NAME_MATCH_WEIGHT) | ||
| + (ref_match * REF_MATCH_WEIGHT) | ||
| + 1 |
There was a problem hiding this comment.
The new weighted rank_expression for unpaid Sales Invoices no longer includes currency_match (it’s still selected as currency_match, but it does not influence ordering anymore). This changes behavior vs. the previous ranking and may cause foreign-currency vouchers to rank similarly to same-currency ones. If currency is still intended to affect ranking, add it back with an explicit weight (or remove it from the select if it’s intentionally no longer used).
| rank_expression = ( | ||
| ref_rank + party_match + currency_match + amount_rank + date_rank + name_match + ref_match + 1 | ||
| (ref_rank * REF_RANK_WEIGHT) | ||
| + (party_match * PARTY_RANK_WEIGHT) | ||
| + (amount_rank * AMOUNT_RANK_WEIGHT) | ||
| + (date_rank * DATE_RANK_WEIGHT) | ||
| + (name_match * NAME_MATCH_WEIGHT) | ||
| + (ref_match * REF_MATCH_WEIGHT) | ||
| + 1 |
There was a problem hiding this comment.
Same issue as unpaid Sales Invoices: currency_match was previously part of the unpaid Purchase Invoice ranking but is now omitted from the weighted rank_expression while still being selected. If currency should still influence the ordering, include it with a weight; otherwise drop the currency_match column to avoid dead fields/misleading output.
| MAX_QUERY_RESULTS = 150 | ||
| # Weights for ranking parameters | ||
| REF_RANK_WEIGHT = 3 # Reference field equality match | ||
| PARTY_RANK_WEIGHT = 2 # Party match | ||
| AMOUNT_RANK_WEIGHT = 2 # Amount match | ||
| DATE_RANK_WEIGHT = 1 # Date match | ||
| NAME_MATCH_WEIGHT = 3 # Name (Paid From) match | ||
| REF_MATCH_WEIGHT = 3 # Reference number found in transaction description |
There was a problem hiding this comment.
Introducing weights changes the overall rank scale (it can now exceed the previously documented max rank). There is existing documentation in the ranking utilities describing a fixed min/max range; consider updating that note (or documenting the new expected range here) so future changes don’t rely on stale assumptions about rank values.
Problem
All parameters (like date, name, amount) etc. had the same strength in for the ranking in "Match Voucher" tab.
Example: Purchase Invoices are rarely paid on the Posting Date, but they are often paid with the exact Grand Total. Currently both parameters have the same effect on the ranking.
Assumption of this PR
In practice a prioritization of the parameters increase the chances of having the correct vouchers further up in the list of vouchers.
Solution
Add hardcoded weights to prioritize the ranking in the Bank Reconciliation Beta (Match Voucher).