Skip to content

Fix flaky RSpec: include local_hcb_code.ledger_item in calculated_ledger_item - #14535

Draft
garyhtou with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-rspec-shard-4-8-error
Draft

Fix flaky RSpec: include local_hcb_code.ledger_item in calculated_ledger_item#14535
garyhtou with Copilot wants to merge 2 commits into
mainfrom
copilot/fix-rspec-shard-4-8-error

Conversation

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary of the problem

bad_settled_mapping_spec.rb fails non-deterministically (reproducible with --seed 17199) with:

RuntimeError: CanonicalTransaction 4 has calculated a different ledger item from its local_hcb_code. ( vs. 6)

CanonicalTransaction and CanonicalPendingTransaction each use HCB-000-{id} as their hcb_code when no linked object exists. Because they draw from independent PostgreSQL sequences, they can coincidentally receive the same id. When that happens, the CPT's assign_ledger_item creates a Ledger::Item and attaches it to the shared HcbCode. When the CT is then created, its assign_ledger_item check sees calculated_ledger_item = nil but local_hcb_code.ledger_item = <CPT's item> and raises via Rails.error.unexpected.

Describe your changes

Added local_hcb_code&.ledger_item as a third fallback in CanonicalTransaction#calculated_ledger_item:

# before
@calculated_ledger_item ||= Ledger::Item.find_by(short_code:) || linked_object_v2&.ledger_item

# after
@calculated_ledger_item ||= Ledger::Item.find_by(short_code:) || linked_object_v2&.ledger_item || local_hcb_code&.ledger_item

This makes CT consistent with CanonicalPendingTransaction#assign_ledger_item, which already uses local_hcb_code.ledger_item as its primary source. The Rails.error.unexpected check is preserved and still fires for genuine mismatches—when short_code or linked_object_v2 resolves to a specific ledger item that disagrees with what the HcbCode stores. The false positive (nil vs. an already-assigned item from a co-grouped record) is eliminated.

…er_item

Co-authored-by: garyhtou <20099646+garyhtou@users.noreply.github.com>
@garyhtou

garyhtou commented Aug 7, 2026

Copy link
Copy Markdown
Member

@copilot My intuition tells me that this is not the right fix because we do not want CanonicalTransaction to depend on HCBCode. We plan to deprecate HCBCode since Ledger::Item is replacing it.

… is present

Co-authored-by: garyhtou <20099646+garyhtou@users.noreply.github.com>

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot My intuition tells me that this is not the right fix because we do not want CanonicalTransaction to depend on HCBCode. We plan to deprecate HCBCode since Ledger::Item is replacing it.

Good point. I've reverted the calculated_ledger_item change so it no longer touches local_hcb_code. Instead, the fix is in the assign_ledger_item check: the mismatch error now only fires when calculated_ledger_item is present but disagrees with local_hcb_code.ledger_item. The false positive (nil vs. an already-assigned item) is eliminated without adding any new dependency on HcbCode to calculated_ledger_item.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants