Merged in bugfix/fix-overzealous-duplication (pull request #597)

Bugfix/fix overzealous duplication

* Refactor duplicate processing logic to scope seen pairs by base page, enhancing clarity and accuracy in deduplication.

* Enhance deduplication tests to include base page identifiers in seen pairs

* Merged main into bugfix/fix-overzealous-duplication


Approved-by: Katon Minhas
This commit is contained in:
Alex Galarce
2025-07-07 15:00:56 +00:00
parent f8f65e28ea
commit 7dfabacbe8
2 changed files with 156 additions and 14 deletions
@@ -467,10 +467,9 @@ def reimbursement_level(
reimbursement_level_fields (FieldSet): A FieldSet object containing field prompts.
all_dataset (dict): # TODO: add docstring here
exhibit_page (str): The page number of the exhibit being processed (for a unique identifier)
seen_pairs (set): A set accumulator to track pairs of SERVICE_TERM and REIMB_TERM to avoid duplicate processing.
This is currently across all exhibits, but could be modified to be per-superexhibit if needed.
(i.e. if a exhibit 23.0 [superexhibit 23.0] has subexhibits 23.0, 23.1, 23.2, etc.,
then the pairs could be tracked per superexhibit.) This is NOT currently implemented.
seen_pairs (set): A set accumulator to track tuples of (base_page, SERVICE_TERM, REIMB_TERM) to avoid duplicate processing.
(i.e. if a exhibit 23.0 [base page 23] has subexhibits 23.0, 23.1, 23.2, etc.,
then the pairs could be tracked per base page.)
Returns:
dict: The parsed LLM response as a list of dictionaries with unique identifiers
@@ -516,13 +515,16 @@ def deduplicate_with_accumulator(
Args:
answers (list[dict]): List of dictionaries with SERVICE_TERM and REIMB_TERM keys.
seen_pairs (set): A set of previously seen (SERVICE_TERM, REIMB_TERM) pairs.
seen_pairs (set): A set of previously seen (base_page, SERVICE_TERM, REIMB_TERM) pairs.
exhibit_page (str): The exhibit page number, for logging.
filename (str): The name of the file being processed, for logging
Returns:
list[dict]: Deduplicated list of dictionaries.
"""
# Extract base page
base_page = exhibit_page.split(".")[0] # e.g. "23.0" -> "23", "23.0.0" -> "23"
deduplicated = []
duplicates_found = 0
@@ -530,8 +532,8 @@ def deduplicate_with_accumulator(
service_term = answer_dict.get("SERVICE_TERM", "").strip()
reimb_term = answer_dict.get("REIMB_TERM", "").strip()
# Create a tuple key for comparison
pair_key = (service_term, reimb_term)
# Create a tuple key scoped to the base page
pair_key = (base_page, service_term, reimb_term)
if pair_key not in seen_pairs:
seen_pairs.add(pair_key) # Update accumulator
@@ -539,12 +541,12 @@ def deduplicate_with_accumulator(
else:
duplicates_found += 1
logging.debug(
f"Cross-exhibit duplicate found in {filename}, exhibit {exhibit_page}: {service_term} - {reimb_term}"
f"Within-base-page duplicate found in {filename}, base page {base_page}, exhibit {exhibit_page}: {service_term} - {reimb_term}"
)
if duplicates_found > 0:
logging.info(
f"Removed {duplicates_found} duplicate pairs from exhibit {exhibit_page} in {filename}"
f"Removed {duplicates_found} duplicate pairs from exhibit {exhibit_page} (base page {base_page}) in {filename}"
)
return deduplicated
+145 -5
View File
@@ -154,15 +154,15 @@ class TestOneToN(unittest.TestCase):
self.assertEqual(len(result), 2)
# Verify seen_pairs was updated
self.assertEqual(len(seen_pairs), 2)
self.assertIn(("Service A", "paid at 80% of fee schedule"), seen_pairs)
self.assertIn(("Service B", "$50 per visit"), seen_pairs)
self.assertIn(("1", "Service A", "paid at 80% of fee schedule"), seen_pairs)
self.assertIn(("1", "Service B", "$50 per visit"), seen_pairs)
@patch('src.investment.one_to_n_funcs.get_reimbursement_primary')
@patch('src.investment.one_to_n_funcs.validate_reimbursements_for_llm')
def test_reimbursement_level_cross_exhibit_deduplication(self, mock_validate_llm, mock_reimbursement_primary):
"""Test that pairs seen in previous exhibits are filtered out"""
# Setup - pre-populate seen_pairs as if previous exhibit processed
seen_pairs = {("Service A", "paid at 80% of fee schedule"),}
seen_pairs = {("2", "Service A", "paid at 80% of fee schedule"),}
mock_validate_llm.return_value = True
# Current exhibit has same pair plus new one
@@ -193,10 +193,150 @@ class TestOneToN(unittest.TestCase):
# Verify both pairs are now in seen_pairs
self.assertEqual(len(seen_pairs), 2)
@patch('src.investment.one_to_n_funcs.get_reimbursement_primary')
@patch('src.investment.one_to_n_funcs.validate_reimbursements_for_llm')
def test_reimbursement_level_cross_exhibit_deduplication_same_base_page(self, mock_validate_llm, mock_reimbursement_primary):
"""Test that pairs seen in previous exhibits within same base page are filtered out"""
# Setup - pre-populate seen_pairs as if previous exhibit in same base page processed
seen_pairs = {("2", "Service A", "paid at 80% of fee schedule"),}
mock_validate_llm.return_value = True
# Current exhibit (2.1) has same pair plus new one - same base page
mock_reimbursement_primary.return_value = [
{"SERVICE_TERM": "Service A", "REIMB_TERM": "paid at 80% of fee schedule"}, # Should be filtered (duplicate in base page 2)
{"SERVICE_TERM": "Service B", "REIMB_TERM": "$50 per visit"} # Should be kept
]
mock_fields = MagicMock()
mock_dataset = {}
exhibit_page = "2.1" # Same base page (2) as seen_pairs
with patch('src.investment.one_to_n_funcs.get_special_cases') as mock_special_cases, \
patch('src.investment.code_funcs.get_code_breakout') as mock_code_breakout:
mock_special_cases.side_effect = lambda x, _: x
mock_code_breakout.side_effect = lambda x, _, __: x
# Execute
result = one_to_n_funcs.reimbursement_level(
self.exhibit_text, self.filename, mock_fields, mock_dataset,
exhibit_page, seen_pairs
)
# Assert - should only process the new pair (Service A filtered out)
self.assertEqual(len(result), 1)
self.assertEqual(result[0]["SERVICE_TERM"], "Service B")
# Verify both pairs are now in seen_pairs
self.assertEqual(len(seen_pairs), 2)
self.assertIn(("2", "Service B", "$50 per visit"), seen_pairs)
@patch('src.investment.one_to_n_funcs.get_reimbursement_primary')
@patch('src.investment.one_to_n_funcs.validate_reimbursements_for_llm')
def test_reimbursement_level_cross_exhibit_deduplication_different_base_page(self, mock_validate_llm, mock_reimbursement_primary):
"""Test that pairs from different base pages are NOT filtered out"""
# Setup - pre-populate seen_pairs as if previous exhibit from different base page processed
seen_pairs = {("23", "Service A", "paid at 80% of fee schedule"),}
mock_validate_llm.return_value = True
# Current exhibit has same pair but from different base page
mock_reimbursement_primary.return_value = [
{"SERVICE_TERM": "Service A", "REIMB_TERM": "paid at 80% of fee schedule"}, # Should be kept (different base page)
{"SERVICE_TERM": "Service B", "REIMB_TERM": "$50 per visit"} # Should be kept
]
mock_fields = MagicMock()
mock_dataset = {}
exhibit_page = "24.0" # Different base page (24) from seen_pairs (23)
with patch('src.investment.one_to_n_funcs.get_special_cases') as mock_special_cases, \
patch('src.investment.code_funcs.get_code_breakout') as mock_code_breakout:
mock_special_cases.side_effect = lambda x, _: x
mock_code_breakout.side_effect = lambda x, _, __: x
# Execute
result = one_to_n_funcs.reimbursement_level(
self.exhibit_text, self.filename, mock_fields, mock_dataset,
exhibit_page, seen_pairs
)
# Assert - should process both pairs (no filtering due to different base page)
self.assertEqual(len(result), 2)
# Verify all pairs are now in seen_pairs (original + 2 new)
self.assertEqual(len(seen_pairs), 3)
self.assertIn(("23", "Service A", "paid at 80% of fee schedule"), seen_pairs) # Original
self.assertIn(("24", "Service A", "paid at 80% of fee schedule"), seen_pairs) # New
self.assertIn(("24", "Service B", "$50 per visit"), seen_pairs) # New
def test_deduplicate_with_accumulator_base_page_extraction(self):
"""Test that base page extraction works correctly for various exhibit page formats"""
answers = [
{"SERVICE_TERM": "Service A", "REIMB_TERM": "term1"},
{"SERVICE_TERM": "Service B", "REIMB_TERM": "term2"}
]
test_cases = [
("23", "23"), # No decimal
("23.0", "23"), # Single decimal
("23.1", "23"), # Single decimal with sub-exhibit
("23.0.0", "23"), # Double decimal
("123.45.67", "123") # Multiple decimals
]
for exhibit_page, expected_base_page in test_cases:
seen_pairs = set()
result = one_to_n_funcs.deduplicate_with_accumulator(
answers, seen_pairs, exhibit_page, "test_file.pdf"
)
# Verify correct base page was used in the keys
expected_keys = {
(expected_base_page, "Service A", "term1"),
(expected_base_page, "Service B", "term2")
}
self.assertEqual(seen_pairs, expected_keys, f"Failed for exhibit_page: {exhibit_page}")
def test_deduplicate_with_accumulator_within_vs_across_base_pages(self):
"""Test deduplication behavior within same vs different base pages"""
answers = [
{"SERVICE_TERM": "Service A", "REIMB_TERM": "term1"},
{"SERVICE_TERM": "Service B", "REIMB_TERM": "term2"}
]
# First exhibit in base page 23
seen_pairs = set()
result1 = one_to_n_funcs.deduplicate_with_accumulator(
answers, seen_pairs, "23.0", "test_file.pdf"
)
self.assertEqual(len(result1), 2) # Both kept
self.assertEqual(len(seen_pairs), 2)
# Second exhibit in same base page 23 - should filter duplicates
result2 = one_to_n_funcs.deduplicate_with_accumulator(
answers, seen_pairs, "23.1", "test_file.pdf"
)
self.assertEqual(len(result2), 0) # Both filtered (duplicates within base page)
self.assertEqual(len(seen_pairs), 2) # No new pairs added
# Third exhibit in different base page 24 - should keep all
result3 = one_to_n_funcs.deduplicate_with_accumulator(
answers, seen_pairs, "24.0", "test_file.pdf"
)
self.assertEqual(len(result3), 2) # Both kept (different base page)
self.assertEqual(len(seen_pairs), 4) # 2 new pairs added
# Verify final state
expected_pairs = {
("23", "Service A", "term1"), ("23", "Service B", "term2"),
("24", "Service A", "term1"), ("24", "Service B", "term2")
}
self.assertEqual(seen_pairs, expected_pairs)
@patch('src.investment.one_to_n_funcs.validate_reimbursements_for_llm')
def test_reimbursement_level_all_duplicates(self, mock_validate_llm):
"""Test behavior when all pairs are duplicates"""
seen_pairs = {("Service A", "paid at 80% of fee schedule"), ("Service B", "$50 per visit")}
seen_pairs = {("3", "Service A", "paid at 80% of fee schedule"), ("3", "Service B", "$50 per visit")}
mock_validate_llm.return_value = True
with patch('src.investment.one_to_n_funcs.get_reimbursement_primary') as mock_primary:
mock_primary.return_value = [
@@ -237,7 +377,7 @@ class TestOneToN(unittest.TestCase):
self.assertEqual(len(seen_pairs), 3)
# Verify stripping works
self.assertIn(("Service C", "$150"), seen_pairs)
self.assertIn(("test_page", "Service C", "$150"), seen_pairs)
if __name__ == '__main__':
unittest.main()