Skip to content

feat(Bank Reconcilitation Beta): prioritize ranking parameters #331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PatrickDEissler wants to merge 5 commits into
base: version-15-hotfix
Choose a base branch
from
Open

feat(Bank Reconcilitation Beta): prioritize ranking parameters #331

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
399882a
feat(Bank Reconcilitation Beta): prioritize ranking parameters
PatrickDEissler Jan 20, 2026
ae40540
refactor: consider pre-commit
PatrickDEissler Jan 20, 2026
f6dba81
Merge branch 'version-15-hotfix' into matching-priority
PatrickDEissler Apr 9, 2026
7519402
docs: clarify comments on hardcoded ranking parameters
PatrickDEissler Apr 9, 2026
137f752
fix: apply REF_MATCH_WEIGHT when reference matches in description
PatrickDEissler Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 78 additions & 12 deletions ..._kosma_integration/doctype/bank_reconciliation_tool_beta/bank_reconciliation_tool_beta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@
from banking.overrides.bank_transaction import CustomBankTransaction

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
Comment on lines 31 to +38
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.


class BankReconciliationToolBeta(Document):
Expand Down Expand Up @@ -607,11 +614,11 @@ def check_matching(
# already covered in DB query
continue

# higher rank if voucher name is in bank transaction
# higher rank if voucher reference appears in bank transaction description
reference_no = voucher["reference_no"]
if reference_no and (reference_no.strip() in transaction.description):
voucher["rank"] += 1
voucher["name_in_desc_match"] = 1
Comment thread
PatrickDEissler marked this conversation as resolved.
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Suggested change
voucher["name_in_desc_match"] = 1
voucher["ref_in_desc_match"] = 1

Copilot uses AI. Check for mistakes.
voucher["rank"] += REF_MATCH_WEIGHT

return sorted(matching_vouchers, key=lambda x: x["rank"], reverse=True)

Expand Down Expand Up @@ -774,7 +781,13 @@ def get_bt_matching_query(exact_match: bool, common_filters: frappe._dict, trans
)
party_rank = frappe.qb.terms.Case().when(party_filter, 1).else_(0)

rank_expression = ref_rank + amount_rank + party_rank + unallocated_rank + 1
rank_expression = (
(ref_rank * REF_RANK_WEIGHT)
+ (amount_rank * AMOUNT_RANK_WEIGHT)
+ (party_rank * PARTY_RANK_WEIGHT)
+ (unallocated_rank * 1)
+ 1
)

query = (
frappe.qb.from_(bt)
Expand Down Expand Up @@ -824,7 +837,12 @@ def get_ld_matching_query(exact_match: bool, common_filters: frappe._dict):
reference_rank = ref_equality_condition(loan_disbursement.reference_number, common_filters.reference_no)
party_rank = frappe.qb.terms.Case().when(matching_party, 1).else_(0)

rank_expression = reference_rank + party_rank + date_rank + 1
rank_expression = (
(reference_rank * REF_RANK_WEIGHT)
+ (party_rank * PARTY_RANK_WEIGHT)
+ (date_rank * DATE_RANK_WEIGHT)
+ 1
)

query = (
frappe.qb.from_(loan_disbursement)
Expand Down Expand Up @@ -873,7 +891,12 @@ def get_lr_matching_query(exact_match: bool, common_filters: frappe._dict):
reference_rank = ref_equality_condition(loan_repayment.reference_number, common_filters.reference_no)
party_rank = frappe.qb.terms.Case().when(matching_party, 1).else_(0)

rank_expression = reference_rank + party_rank + date_rank + 1
rank_expression = (
(reference_rank * REF_RANK_WEIGHT)
+ (party_rank * PARTY_RANK_WEIGHT)
+ (date_rank * DATE_RANK_WEIGHT)
+ 1
)

query = (
frappe.qb.from_(loan_repayment)
Expand Down Expand Up @@ -944,7 +967,13 @@ def get_pe_matching_query(
date_condition = Coalesce(pe.reference_date, pe.posting_date) == common_filters.date
date_rank = frappe.qb.terms.Case().when(date_condition, 1).else_(0)

rank_expression = ref_rank + amount_rank + party_rank + date_rank + 1
rank_expression = (
(ref_rank * REF_RANK_WEIGHT)
+ (amount_rank * AMOUNT_RANK_WEIGHT)
+ (party_rank * PARTY_RANK_WEIGHT)
+ (date_rank * DATE_RANK_WEIGHT)
+ 1
)

query = (
frappe.qb.from_(pe)
Expand Down Expand Up @@ -1047,7 +1076,9 @@ def get_je_matching_query(
)
date_condition = subquery.reference_date == common_filters.date
date_rank = frappe.qb.terms.Case().when(date_condition, 1).else_(0)
rank_expression = ref_rank + amount_rank + date_rank + 1
rank_expression = (
(ref_rank * REF_RANK_WEIGHT) + (amount_rank * AMOUNT_RANK_WEIGHT) + (date_rank * DATE_RANK_WEIGHT) + 1
)

query = (
frappe.qb.from_(subquery)
Expand Down Expand Up @@ -1106,7 +1137,15 @@ def get_si_matching_query(
else Cast(0, "int")
)

rank_expression = ref_rank + party_rank + amount_rank + date_rank + name_match + ref_match + 1
rank_expression = (
(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
)

query = (
frappe.qb.from_(sip)
Expand Down Expand Up @@ -1209,7 +1248,13 @@ def get_unpaid_si_matching_query(
date_rank = frappe.qb.terms.Case().when(date_condition, 1).else_(0)

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
Comment on lines 1250 to +1257
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
)

query = (
Expand Down Expand Up @@ -1302,7 +1347,15 @@ def get_pi_matching_query(
else Cast(0, "int")
)

rank_expression = ref_rank + party_rank + amount_rank + date_rank + name_match + ref_match + 1
rank_expression = (
(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
)

query = (
frappe.qb.from_(purchase_invoice)
Expand Down Expand Up @@ -1412,7 +1465,13 @@ def get_unpaid_pi_matching_query(
date_rank = frappe.qb.terms.Case().when(date_condition, 1).else_(0)

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
Comment on lines 1467 to +1474
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
)

query = (
Expand Down Expand Up @@ -1498,7 +1557,14 @@ def get_unpaid_ec_matching_query(
else Cast(0, "int")
)

rank_expression = ref_rank + party_match + amount_rank + name_match + ref_match + 1
rank_expression = (
(ref_rank * REF_RANK_WEIGHT)
+ (party_match * PARTY_RANK_WEIGHT)
+ (amount_rank * AMOUNT_RANK_WEIGHT)
+ (name_match * NAME_MATCH_WEIGHT)
+ (ref_match * REF_MATCH_WEIGHT)
+ 1
)

query = (
frappe.qb.from_(expense_claim)
Expand Down
Loading