feat(PE): carry advance tax through invoice consumption

[HenningWendtland]

Fixes #42843

Rebased on current develop. Two commits: the first is an independent bug fix that stands on its own, the second is the feature.

Issue Addressed

In Germany, the EU, and other VAT jurisdictions, taxes are due upon receipt of an advance payment, not only on the invoice that later consumes it. ERPNext's Payment Entry can already carry tax rows, but the rest of the advance-consumption pipeline did not honour them.

Sales Order 1 190 (1 000 net + 19 % VAT), advance payment of 1 190, final invoice 1 190:

	VAT liability booked	Invoice outstanding
develop	380 € (twice)	190 € — Partly Paid
this PR	190 €	0 € — Paid

Tracing the problem from the end:

• Tax reversal on consumption was missing. When a Sales Invoice with an allocated advance is submitted, ERPNext rewrites the Payment Entry's reference (SO → SI) and posts GL moving the net advance to receivables. The tax portion was forgotten, so it stayed outstanding and the invoice booked the same VAT again. This PR posts additional GL legs moving the tax collected at PE submission from the tax account to the party account, per tax account. Cancellation and amendment of the SI reverse those entries cleanly.
• allocated_amount is net under included_in_paid_amount=1. A parallel allocated_gross_amount on Payment Entry Reference, Sales Invoice Advance and Purchase Invoice Advance carries the gross forward, plus unallocated_gross_amount on Payment Entry for the remainder a later invoice may consume.
• Multi-tax / multi-reference allocation. Each tax row is split proportionally by allocated_amount, anchored on the net paid amount rather than on the sum of current allocations. A reference's share therefore depends only on its own allocation and survives reference rewriting, partial consumption and unlinking. The rounding remainder lands on the last reference only when the payment is fully allocated.
• The Payment Entry could not even be saved. With an included_in_paid_amount tax row, allocate_amount_to_references distributed the gross while set_unallocated_amount subtracted the tax, so the entry over-allocated by exactly the tax and failed validation with Difference Amount must be zero. That is the first commit, and it is independent of everything above.

Key Additions

Backend:

• PaymentEntry.compute_advance_tax_breakdown — returns {ref_row_name: {account_head: tax_amount}}, the per-reference split of the PE's advance taxes.
• PaymentEntry.set_allocated_gross_amount — persists allocated_gross_amount per reference and unallocated_gross_amount on the entry. Called from validate() and from update_reference_in_payment_entry after the consume rewrites references.
• PaymentEntry._add_advance_tax_reversal_for_reference — posts the per-account tax-reversal legs on consumption, and emits zero-amount cancel templates so SI/PI cancel and amend reverse them via make_reverse_gl_entries(partial_cancel=True).
• set_advances (in accounts/services/advances.py) and calculate_total_advance (in taxes_and_totals.py) — drive invoice consumption off gross, with a net fallback.
• PaymentEntry.allocate_amount_to_references — subtracts included taxes before splitting across references, recomputing the tax rows first since they may still reflect the previous paid amount.

Cross-cutting fix:

• make_reverse_gl_entries(partial_cancel=True) now matches nullable columns with IS NULL rather than =. The new tax legs carry no party_type/party, and NULL = NULL is false in SQL, so the cancel update would silently match nothing.

Frontend:

• refresh_allocated_gross_amounts in payment_entry.js keeps the read-only gross preview in sync while editing; the server remains the source of truth at save time.
• The Advance Taxes and Charges handlers now re-run allocation, so editing a tax row settles the form instead of leaving it unsaveable.

Scope

• Only applies to advances booked in a separate party account (book_advance_payments_in_separate_party_account). That is the only path with a consumption GL event to post the reversal on, so set_advances keeps allocating on net otherwise — behaviour is unchanged for every other company.
• Only Sales Invoice / Purchase Invoice references get a reversal; Journal Entry and reverse-payment references are untouched.
• Deduct rows and tax-withholding rows are excluded: they are withheld from the payment rather than collected on top of it.
• The Sales Order's advance_paid still reports the net, as it comes from the Advance Payment Ledger. Left alone to keep the diff contained.

Backwards compatibility

With no advance taxes, allocated_gross_amount == allocated_amount on every path and no extra GL entries are emitted. Reads use ... or allocated_amount, so legacy rows (NULL/0) are handled transparently with no migration step.

Screenshots

PE Document with new fields

SI Totals with Total Advance filled from Allocated Gross Advance

PE after SI submission with partial consumption

GL Entries for PE after reposting (includes tax handling now)

GL Entries for PE after cancellation of the SI

Multi-currency

USD advance in a EUR company at 0.9137. calculate_taxes writes tax_amount and base_tax_amount with the same company-currency value, so the breakdown is built in EUR and converted back to the party currency for the receivable leg. The gross/net split stays in USD:

Ledger for that entry: receivable credited 1 190,00 USD = 1 087,30 EUR, advance account and tax account both net to zero, total Dr = total Cr.

Out of Scope

Cancelling a Sales Invoice that partially consumed an advance does not bump the allocated amount back up on the Payment Entry reference row. Pre-existing, untouched here, worth its own fix.

Follow-Up PR

Carrying tax rows over automatically from the "Create → Payment" button on SO/PO. Kept separate to hold this diff down; it depends on the gross/net model landing first.

Tests

19 new tests in test_payment_entry.py: gross computation, proportional split across references, multi-currency in both Receive and Pay direction including the reversal GL itself, tax-reversal GL on consume and on cancel, unlinking, partial and unallocated consumption, Deduct and withholding exclusion, and the no-separate-account case.

test_payment_entry, test_sales_invoice, test_purchase_invoice, test_payment_reconciliation and test_unreconcile_payment were run against this branch and against develop on the same machine, with identical results. Also verified manually on a German chart of accounts: advance creation from a Sales Order, partial consumption, invoice cancellation, payment cancellation.

Update

Three follow-up fixes after self-review:

• Editing an advance tax row no longer clears reference amounts that were allocated by hand.
• unallocated_gross_amount is grossed up by advance tax only, like the reference rows, so the same money is worth the same whether it sits allocated or unallocated. A Deduct row is withheld from the payment and already inside the net, so it grosses nothing up.
• The stored gross now rounds the way the reversal GL legs do; multi-currency entries with several references were left a cent outstanding.

https://docs.frappe.io/wiki/change-requests/6tbus2aj7k

[codecov]

Codecov Report

❌ Patch coverage is 98.05014% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.86%. Comparing base (2773b7c) to head (7253b16).
⚠️ Report is 219 commits behind head on develop.

Files with missing lines	Patch %	Lines
...xt/accounts/doctype/payment_entry/payment_entry.py	93.54%	6 Missing ⚠️
erpnext/controllers/accounts_controller.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #54921      +/-   ##
===========================================
+ Coverage    79.75%   79.86%   +0.11%     
===========================================
  Files         1170     1170              
  Lines       128096   128444     +348     
===========================================
+ Hits        102167   102586     +419     
+ Misses       25929    25858      -71     

Files with missing lines	Coverage Δ
...counts/doctype/payment_entry/test_payment_entry.py	99.75% <100.00%> (+0.05%)	⬆️
...payment_entry_reference/payment_entry_reference.py	88.88% <ø> (ø)
...rchase_invoice_advance/purchase_invoice_advance.py	100.00% <ø> (ø)
...ype/sales_invoice_advance/sales_invoice_advance.py	100.00% <ø> (ø)
erpnext/accounts/general_ledger.py	93.31% <100.00%> (+0.03%)	⬆️
erpnext/accounts/utils.py	74.29% <100.00%> (+0.08%)	⬆️
erpnext/controllers/taxes_and_totals.py	95.55% <100.00%> (+0.03%)	⬆️
erpnext/controllers/accounts_controller.py	85.47% <88.88%> (+0.02%)	⬆️
...xt/accounts/doctype/payment_entry/payment_entry.py	83.46% <93.54%> (+5.57%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Payment entries now persist per-reference gross allocations (allocated_gross_amount) that include advance taxes marked included_in_paid_amount. Client-side handlers compute a gross/net ratio to preview and refresh per-reference gross amounts after allocations or tax changes. Server-side adds compute_advance_tax_breakdown() and set_allocated_gross_amount(), updates allocation to subtract included taxes before distributing paid amounts, persists unallocated_gross_amount, and emits per-reference advance-tax reversal GL legs. Doctype schemas, invoice controllers, totals logic, utilities, GL cancellation behavior, and tests are updated to be gross-aware.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

needs-tests

Suggested reviewers

• ruthra-kumar
• khushi8112

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.76% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title clearly identifies the main change: advancing tax handling through invoice consumption workflow, which is the primary objective of the PR.
Description check	✅ Passed	Description comprehensively relates to the changeset, explaining the tax-flow problem, solutions (allocated_gross_amount, proportional splitting, GL reversal), backwards compatibility, and linking to issue #42843.
Linked Issues check	✅ Passed	Changes comprehensively address issue #42843 objectives: allocated_gross_amount carries gross figures through consumption [all files], proportional tax allocation across references [payment_entry.py], tax-reversal GL on invoice consume [payment_entry.py, general_ledger.py], Sales Invoice total_advance uses gross with fallback [taxes_and_totals.py, accounts_controller.py], backwards compatibility maintained via OR fallbacks [throughout].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #42843: new allocated_gross_amount fields, proportional tax breakdown, tax-reversal GL posting, frontend sync logic, and enabling changes (NULL handling in partial_cancel). Follow-up UI improvements and pre-existing cancel-on-partial-consume bug are explicitly noted as out-of-scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

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.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 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 `@erpnext/accounts/doctype/payment_entry/payment_entry.js`:
- Around line 1132-1144: In get_gross_net_ratio, stop using tax.tax_amount and
instead use tax.base_tax_amount (like get_included_taxes does), converting
base_tax_amount into the party/account currency by dividing by the appropriate
exchange rate before summing: if frm.doc.payment_type === "Pay" use
target_exchange_rate, if "Receive" use source_exchange_rate (matching the
pattern in calculate_taxes); continue to respect tax.included_in_paid_amount and
tax.add_deduct_tax when accumulating included_taxes, then compute net =
flt(frm.doc.paid_amount) - included_taxes and return net ? paid / net : 1.

In `@erpnext/accounts/doctype/payment_entry/payment_entry.py`:
- Around line 716-778: compute_advance_tax_breakdown is adding company‑currency
tax_amounts to reference (party) amounts causing FX drift; convert advance-tax
shares into the reference/party currency before summing. Update
compute_advance_tax_breakdown (and callers like set_allocated_gross_amount and
get_included_taxes when in_account_currency=True) to, for each tax share
computed from tax.tax_amount, convert that company‑currency value into the
reference/party currency using the appropriate exchange rate helper (e.g.
frappe.get_exchange_rate or your existing payment-entry currency conversion
utility) keyed by company vs reference currency, then distribute/round the
converted shares and use those converted numbers when setting
ref.allocated_gross_amount. Ensure you reference tax.tax_amount and
ref.allocated_amount when performing the conversion so all math is done in the
same currency.

In `@erpnext/accounts/utils.py`:
- Line 804: The unlink flow in remove_ref_doc_link_from_pe() currently removes
reference rows and calls set_amounts() but doesn't recalculate allocated gross,
leaving allocated_gross_amount stale; after the existing set_amounts() call
(inside remove_ref_doc_link_from_pe) invoke
payment_entry.set_allocated_gross_amount() (same method used in the
link/allocate paths) so surviving reference rows get their
allocated_gross_amount recalculated and persisted before returning/committing.

In `@erpnext/controllers/accounts_controller.py`:
- Around line 1531-1533: The allocation logic can produce negative allocations
due to floating-point drift when computing remaining = amount -
advance_allocated_gross; update the block in accounts allocation (variables:
allocated_gross_amount, allocated_amount, advance_allocated_gross) to clamp
remaining to at least 0 before taking min with source_gross (i.e., use remaining
= max(0, amount - advance_allocated_gross) and then allocated_gross_amount =
min(remaining, source_gross)), then compute allocated_amount =
allocated_gross_amount * source_net / source_gross if source_gross else 0 and
add flt(allocated_gross_amount) to advance_allocated_gross.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 8755733d-47ff-4a68-a104-3267413e1abe

📥 Commits

Reviewing files that changed from the base of the PR and between 45f05fb and f26fd80.

📒 Files selected for processing (14)

• erpnext/accounts/doctype/payment_entry/payment_entry.js
• erpnext/accounts/doctype/payment_entry/payment_entry.py
• erpnext/accounts/doctype/payment_entry/test_payment_entry.py
• erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.json
• erpnext/accounts/doctype/payment_entry_reference/payment_entry_reference.py
• erpnext/accounts/doctype/purchase_invoice_advance/purchase_invoice_advance.json
• erpnext/accounts/doctype/purchase_invoice_advance/purchase_invoice_advance.py
• erpnext/accounts/doctype/sales_invoice_advance/sales_invoice_advance.json
• erpnext/accounts/doctype/sales_invoice_advance/sales_invoice_advance.py
• erpnext/accounts/general_ledger.py
• erpnext/accounts/utils.py
• erpnext/controllers/accounts_controller.py
• erpnext/controllers/taxes_and_totals.py
• erpnext/public/js/controllers/taxes_and_totals.js

[HenningWendtland]

Outside diff comments:
In @erpnext/controllers/accounts_controller.py:

• Around line 3413-3440: In the include_unallocated branch, also return the
  gross remainder so set_advances can use it: update the q.select call (after
  get_common_query) to select both payment_entry.unallocated_amount.as_("amount")
  and payment_entry.unallocated_gross_amount and alias the gross field to the same
  name used by the allocated branch (allocated_gross_amount) so the consumer
  (set_advances) sees gross values; i.e., modify the select in the
  include_unallocated branch to include payment_entry.unallocated_gross_amount
  (aliased to allocated_gross_amount) alongside payment_entry.unallocated_amount,
  leaving payment_entries accumulation and the rest of the logic unchanged.

I think Coderabbit flags another pre-existing bug here: That taxes are not properly resolved for PEs with taxes
and without allocation (no / partial referene allocation). This is not introduced by my PR and I would suggest fixing this also in the future

UPDATE: fixed in 5d667e65d667e6

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

erpnext/accounts/doctype/payment_entry/test_payment_entry.py (2)

357-365: 💤 Low value

Capture the book_advance_payments_in_separate_party_account flag too, not just the advance account.

The try/finally only snapshots default_advance_received_account. The flag itself is unconditionally reset to 0 in the finally block, so if a future test ordering (or shared setup) leaves this Company flag enabled, this test will silently flip it off and leak that to subsequent tests. The same gap exists in test_advance_tax_reverses_on_si_cancel (lines 446–454, 514–522).

♻️ Symmetric snapshot/restore

-		previous_advance_account = frappe.db.get_value("Company", company, "default_advance_received_account")
+		previous_book_in_separate_account, previous_advance_account = frappe.db.get_value(
+			"Company",
+			company,
+			["book_advance_payments_in_separate_party_account", "default_advance_received_account"],
+		)
 		frappe.db.set_value(
 			"Company",
 			company,
 			{
 				"book_advance_payments_in_separate_party_account": 1,
 				"default_advance_received_account": advance_account,
 			},
 		)
 		try:
 			...
 		finally:
 			frappe.db.set_value(
 				"Company",
 				company,
 				{
-					"book_advance_payments_in_separate_party_account": 0,
+					"book_advance_payments_in_separate_party_account": previous_book_in_separate_account,
 					"default_advance_received_account": previous_advance_account,
 				},
 			)

Also applies to: 422-429

🤖 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 `@erpnext/accounts/doctype/payment_entry/test_payment_entry.py` around lines
357 - 365, The finally block only restores previous_advance_account but
unconditionally resets book_advance_payments_in_separate_party_account to 0;
capture the original flag value (e.g., previous_book_advance_flag =
frappe.db.get_value("Company", company,
"book_advance_payments_in_separate_party_account")) before setting it, and in
the finally restore both values (default_advance_received_account and
book_advance_payments_in_separate_party_account) using frappe.db.set_value so
the test (including test_advance_tax_reverses_on_si_cancel and other spots where
you set book_advance_payments_in_separate_party_account and
default_advance_received_account) does not mutate global Company state for
subsequent tests.

---

735-739: 💤 Low value

Inline comment seems inverted — ~847 is the fixed value, not the pre-fix proportional share.

The assertion below expects so2.allocated_gross_amount ≈ so2_allocated + tax_total ≈ 622.56 + 224.83 ≈ 847.39, which is the post-fix "full tax absorbed" outcome. The pre-fix/buggy stale value is the original proportional share, ≈ 622.56 + (622.56/1322.56)*224.83 ≈ 728.39, not ~847. The current wording reads as if ~847 were the buggy value the new loop avoids, which is backwards.

📝 Suggested rewording

-		# Surviving row: allocated_amount unchanged, allocated_gross_amount absorbs
-		# the full tax (it's the only ref left in the breakdown). Without the new
-		# `frappe.db.set_value` loop in `remove_ref_doc_link_from_pe`, this would
-		# still be the pre-unlink proportional share (~847 — net + ref's old tax
-		# slice), not the full gross.
+		# Surviving row: allocated_amount unchanged, allocated_gross_amount absorbs
+		# the full tax (~847.39 = 622.56 net + 224.83 full tax) because it's the
+		# only ref left in the breakdown. Without the new `frappe.db.set_value`
+		# loop in `remove_ref_doc_link_from_pe`, this would still be the pre-unlink
+		# proportional share (~728 = net + ref's old tax slice).

🤖 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 `@erpnext/accounts/doctype/payment_entry/test_payment_entry.py` around lines
735 - 739, The inline comment in test_payment_entry.py misstates which value is
the pre-fix vs post-fix outcome; update the comment near the assertion involving
so2.allocated_gross_amount (and variables so2_allocated, tax_total) to say that
~847.39 is the post-fix “full tax absorbed” result and that the pre-fix buggy
proportional share was ~728.39, and mention that the frappe.db.set_value loop
added to remove_ref_doc_link_from_pe causes the surviving row to absorb the full
tax rather than the old proportional slice.

🤖 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 `@erpnext/accounts/doctype/payment_entry/payment_entry.js`:
- Around line 1132-1147: The gross/net ratio calculation in get_gross_net_ratio
uses frm.doc.paid_amount (source currency) while included_taxes are converted
using exchange_rate for "Pay", causing mismatch; update the paid value so that
when frm.doc.payment_type === "Pay" you convert paid_amount into the party
(target) currency using the same exchange_rate before computing net (e.g.,
compute paid_in_party = paid_amount / exchange_rate for "Pay"), then use that
converted paid when computing net and returning the ratio.

---

Nitpick comments:
In `@erpnext/accounts/doctype/payment_entry/test_payment_entry.py`:
- Around line 357-365: The finally block only restores previous_advance_account
but unconditionally resets book_advance_payments_in_separate_party_account to 0;
capture the original flag value (e.g., previous_book_advance_flag =
frappe.db.get_value("Company", company,
"book_advance_payments_in_separate_party_account")) before setting it, and in
the finally restore both values (default_advance_received_account and
book_advance_payments_in_separate_party_account) using frappe.db.set_value so
the test (including test_advance_tax_reverses_on_si_cancel and other spots where
you set book_advance_payments_in_separate_party_account and
default_advance_received_account) does not mutate global Company state for
subsequent tests.
- Around line 735-739: The inline comment in test_payment_entry.py misstates
which value is the pre-fix vs post-fix outcome; update the comment near the
assertion involving so2.allocated_gross_amount (and variables so2_allocated,
tax_total) to say that ~847.39 is the post-fix “full tax absorbed” result and
that the pre-fix buggy proportional share was ~728.39, and mention that the
frappe.db.set_value loop added to remove_ref_doc_link_from_pe causes the
surviving row to absorb the full tax rather than the old proportional slice.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 74b962b0-6e77-46f1-b2d3-bca68afb01b8

📥 Commits

Reviewing files that changed from the base of the PR and between dc076d2 and ee51219.

📒 Files selected for processing (2)

• erpnext/accounts/doctype/payment_entry/payment_entry.js
• erpnext/accounts/doctype/payment_entry/test_payment_entry.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

erpnext/accounts/doctype/payment_entry/payment_entry.js (1)

385-412: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Bind unallocated_gross_amount to party currency in dynamic labels/options.

unallocated_gross_amount is not included in the form-level currency metadata update, so it can display with the wrong currency context on multi-currency entries.

Suggested fix

 		frm.set_currency_labels(
-			["total_allocated_amount", "unallocated_amount", "total_taxes_and_charges"],
+			[
+				"total_allocated_amount",
+				"unallocated_amount",
+				"unallocated_gross_amount",
+				"total_taxes_and_charges",
+			],
 			party_account_currency
 		);

 		var currency_field =
 			frm.doc.payment_type == "Receive" ? "paid_from_account_currency" : "paid_to_account_currency";
 		frm.set_df_property("total_allocated_amount", "options", currency_field);
 		frm.set_df_property("unallocated_amount", "options", currency_field);
+		frm.set_df_property("unallocated_gross_amount", "options", currency_field);
 		frm.set_df_property("total_taxes_and_charges", "options", currency_field);

🤖 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 `@erpnext/accounts/doctype/payment_entry/payment_entry.js` around lines 385 -
412, The dynamic currency binding omits "unallocated_gross_amount", causing it
to show the wrong currency; update the two places where currency-linked fields
are listed—add "unallocated_gross_amount" to the array passed into
frm.set_df_property (the ["total_amount", "outstanding_amount",
"allocated_amount", "allocated_gross_amount"] list) and include it in the loop
that calls reference_grid.update_docfield_property(fieldname, "options",
party_currency) so that "unallocated_gross_amount" is bound to party_currency
like the others (refs: frm.set_df_property, party_currency, reference_grid,
update_docfield_property).

🧹 Nitpick comments (1)

erpnext/accounts/doctype/payment_entry/test_payment_entry.py (1)

339-429: 💤 Low value

Consider extracting the repeated advance-account company-setting setup/teardown into a helper.

Three of the new tests duplicate the same ~17-line scaffold to flip book_advance_payments_in_separate_party_account + default_advance_received_account, run the case, and restore in finally. A small context manager (or a setUp/tearDown per test class group) would cut ~50 lines and make the intent of each test stand out. Also worth giving each test its own advance-account name (the consume and cancel tests currently share "Advances Received For Tax Test"), and capturing the original book_advance_payments_in_separate_party_account instead of hardcoding 0 on restore — the latter is just inheriting an existing pattern in the file though.

♻️ Sketch of a context-manager helper

from contextlib import contextmanager

`@contextmanager`
def _separate_advance_account(company: str, advance_account: str):
    prev_flag = cint(frappe.db.get_value("Company", company, "book_advance_payments_in_separate_party_account"))
    prev_acc = frappe.db.get_value("Company", company, "default_advance_received_account")
    frappe.db.set_value(
        "Company",
        company,
        {
            "book_advance_payments_in_separate_party_account": 1,
            "default_advance_received_account": advance_account,
        },
    )
    try:
        yield
    finally:
        frappe.db.set_value(
            "Company",
            company,
            {
                "book_advance_payments_in_separate_party_account": prev_flag,
                "default_advance_received_account": prev_acc,
            },
        )

Then each test becomes:

advance_account = create_account(..., account_name="Advances Received For SI Consume Tax Test", ...)
with _separate_advance_account("_Test Company", advance_account):
    ...

That also gives a natural home for cint (line 7), which I don't see referenced in any of the new tests right now.

Also applies to: 431-522, 731-803

🤖 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 `@erpnext/accounts/doctype/payment_entry/test_payment_entry.py` around lines
339 - 429, Extract the repeated company advance-account setup/teardown into a
reusable helper (e.g. a contextmanager named _separate_advance_account(company,
advance_account)) and use it from test_advance_tax_reversal_on_si_consume (and
the other two tests noted) instead of duplicating the
frappe.db.set_value/restore block; the helper should read and save the current
book_advance_payments_in_separate_party_account flag and
default_advance_received_account (use cint when reading the flag), set the
1/advance_account values, yield, and restore the original values in finally, and
update the tests to create unique advance account names via create_account and
call the helper around the test logic instead of hardcoding a restore value of
0.

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

Outside diff comments:
In `@erpnext/accounts/doctype/payment_entry/payment_entry.js`:
- Around line 385-412: The dynamic currency binding omits
"unallocated_gross_amount", causing it to show the wrong currency; update the
two places where currency-linked fields are listed—add
"unallocated_gross_amount" to the array passed into frm.set_df_property (the
["total_amount", "outstanding_amount", "allocated_amount",
"allocated_gross_amount"] list) and include it in the loop that calls
reference_grid.update_docfield_property(fieldname, "options", party_currency) so
that "unallocated_gross_amount" is bound to party_currency like the others
(refs: frm.set_df_property, party_currency, reference_grid,
update_docfield_property).

---

Nitpick comments:
In `@erpnext/accounts/doctype/payment_entry/test_payment_entry.py`:
- Around line 339-429: Extract the repeated company advance-account
setup/teardown into a reusable helper (e.g. a contextmanager named
_separate_advance_account(company, advance_account)) and use it from
test_advance_tax_reversal_on_si_consume (and the other two tests noted) instead
of duplicating the frappe.db.set_value/restore block; the helper should read and
save the current book_advance_payments_in_separate_party_account flag and
default_advance_received_account (use cint when reading the flag), set the
1/advance_account values, yield, and restore the original values in finally, and
update the tests to create unique advance account names via create_account and
call the helper around the test logic instead of hardcoding a restore value of
0.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: cc218a3e-1e54-45c9-be61-45182c6bd429

📥 Commits

Reviewing files that changed from the base of the PR and between ee51219 and a4a89bb.

📒 Files selected for processing (5)

• erpnext/accounts/doctype/payment_entry/payment_entry.js
• erpnext/accounts/doctype/payment_entry/payment_entry.json
• erpnext/accounts/doctype/payment_entry/payment_entry.py
• erpnext/accounts/doctype/payment_entry/test_payment_entry.py
• erpnext/controllers/accounts_controller.py

🚧 Files skipped from review as they are similar to previous changes (1)

• erpnext/accounts/doctype/payment_entry/payment_entry.py

[HenningWendtland]

bump

[codecov-commenter]

Codecov Report

❌ Patch coverage is 98.05014% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.86%. Comparing base (2773b7c) to head (7253b16).
⚠️ Report is 2829 commits behind head on develop.

Files with missing lines	Patch %	Lines
...xt/accounts/doctype/payment_entry/payment_entry.py	93.54%	6 Missing ⚠️
erpnext/controllers/accounts_controller.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #54921      +/-   ##
===========================================
+ Coverage    79.75%   79.86%   +0.11%     
===========================================
  Files         1170     1170              
  Lines       128096   128444     +348     
===========================================
+ Hits        102167   102586     +419     
+ Misses       25929    25858      -71     

Files with missing lines	Coverage Δ
...counts/doctype/payment_entry/test_payment_entry.py	99.75% <100.00%> (+0.05%)	⬆️
...payment_entry_reference/payment_entry_reference.py	88.88% <ø> (ø)
...rchase_invoice_advance/purchase_invoice_advance.py	100.00% <ø> (ø)
...ype/sales_invoice_advance/sales_invoice_advance.py	100.00% <ø> (ø)
erpnext/accounts/general_ledger.py	93.31% <100.00%> (+0.03%)	⬆️
erpnext/accounts/utils.py	74.29% <100.00%> (+0.08%)	⬆️
erpnext/controllers/taxes_and_totals.py	95.55% <100.00%> (+0.03%)	⬆️
erpnext/controllers/accounts_controller.py	85.47% <88.88%> (+0.02%)	⬆️
...xt/accounts/doctype/payment_entry/payment_entry.py	83.46% <93.54%> (+5.57%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[greptile-apps]

Confidence Score: 4/5

• Safe to merge once the currency-mismatch issues flagged in prior review rounds (Pay-direction PE using paid_amount instead of received_amount in compute_advance_tax_breakdown) are resolved; the new GL accounting logic and partial-cancel fix are otherwise correct.
• The core mechanics — proportional tax breakdown, partial-cancel NULL-matching fix, gross-field propagation through reconciliation and unlink — are well-reasoned and backed by 19 targeted tests. The remaining open threads are the Pay-direction currency mismatch from prior review (paid_amount vs received_amount as denominator), a silent NULL-propagation in get_unconsumed_advance_tax for legacy PE rows, and a negative-net edge case in get_gross_net_ratio that bypasses the zero guard. None of these block the feature for the stated VAT use case (Receive-direction, German chart of accounts), but the currency mismatch would produce wrong GL values for Pay-direction advances in multi-currency setups.
• erpnext/accounts/doctype/payment_entry/payment_entry.py — specifically the compute_advance_tax_breakdown and set_allocated_gross_amount methods for Pay-direction currency handling (flagged in prior review), and get_gross_net_ratio for the negative-net guard.

_{Reviews (7): Last reviewed commit: "Merge branch 'develop' into improve-adva..." | Re-trigger Greptile}