From 5e8d58b4b4d3c61a59bfa26c0fd6044c49db5bdc Mon Sep 17 00:00:00 2001 From: Tyler Spangler <142833783+tyler-spangler6@users.noreply.github.com> Date: Mon, 2 Dec 2024 13:50:07 -0700 Subject: [PATCH 1/4] add parameter to track if testing claim --- domain-cc/cc-app/src/python_src/pydantic_models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/domain-cc/cc-app/src/python_src/pydantic_models.py b/domain-cc/cc-app/src/python_src/pydantic_models.py index 30048fd41..c765f0736 100644 --- a/domain-cc/cc-app/src/python_src/pydantic_models.py +++ b/domain-cc/cc-app/src/python_src/pydantic_models.py @@ -37,6 +37,7 @@ class VaGovClaim(BaseModel): claim_id: int form526_submission_id: int contentions: conlist(Contention, min_length=1) + testing_claim: Optional[bool] = False class ClassifiedContention(BaseModel): From d70cd14541c1b51331051c70ff84899abc2087cc Mon Sep 17 00:00:00 2001 From: Tyler Spangler <142833783+tyler-spangler6@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:03:34 -0700 Subject: [PATCH 2/4] added logging and updated tests --- domain-cc/cc-app/src/python_src/api.py | 2 + domain-cc/cc-app/tests/test_logging.py | 83 ++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/domain-cc/cc-app/src/python_src/api.py b/domain-cc/cc-app/src/python_src/api.py index 5eed7c1e3..07a80e7ad 100644 --- a/domain-cc/cc-app/src/python_src/api.py +++ b/domain-cc/cc-app/src/python_src/api.py @@ -143,6 +143,7 @@ def log_contention_stats( "is_lookup_table_match": classification_code is not None, "is_multi_contention": is_multi_contention, "endpoint": request.url.path, + "testing_claim": claim.testing_claim, } if request.url.path == "/expanded-contention-classification": @@ -174,6 +175,7 @@ def log_claim_stats_v2( "num_processed_contentions": response.num_processed_contentions, "num_classified_contentions": response.num_classified_contentions, "endpoint": request.url.path, + "testing_claim": claim.testing_claim, } ) diff --git a/domain-cc/cc-app/tests/test_logging.py b/domain-cc/cc-app/tests/test_logging.py index cfd724729..df032da40 100644 --- a/domain-cc/cc-app/tests/test_logging.py +++ b/domain-cc/cc-app/tests/test_logging.py @@ -68,6 +68,7 @@ def test_log_contention_stats_expanded(mocked_func): "is_lookup_table_match": True, "is_multi_contention": False, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": "acl tear", } @@ -107,6 +108,7 @@ def test_non_classified_contentions(mocked_func): "is_lookup_table_match": False, "is_multi_contention": False, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": None, } mocked_func.assert_called_once_with(expected_log) @@ -157,6 +159,7 @@ def test_multiple_contentions(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": "tinnitus ringing hissing ears", }, { @@ -170,6 +173,7 @@ def test_multiple_contentions(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": "anxiety", }, ] @@ -230,6 +234,7 @@ def test_contentions_with_pii(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": None, }, { @@ -243,6 +248,7 @@ def test_contentions_with_pii(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": None, }, ] @@ -305,6 +311,7 @@ def test_log_claim_stats(mocked_func): "num_processed_contentions": 2, "num_classified_contentions": 1, "endpoint": "/expanded-contention-classification", + "testing_claim": False, } log_claim_stats_v2(test_claim, classifier_response, test_expanded_request) mocked_func.assert_called_once_with(expected_log) @@ -346,6 +353,7 @@ def test_current_classifier_contention(mocked_func): "is_lookup_table_match": False, "is_multi_contention": False, "endpoint": "/va-gov-claim-classifier", + "testing_claim": False, } mocked_func.assert_called_once_with(expected_logging_dict) @@ -396,6 +404,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": None, }, { @@ -409,6 +418,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", + "testing_claim": False, "processed_contention_text": "anxiety", }, ] @@ -429,6 +439,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "num_processed_contentions": 2, "num_classified_contentions": 1, "endpoint": "/expanded-contention-classification", + "testing_claim": False, } for i in range(len(test_contentions)): @@ -443,3 +454,75 @@ def test_full_logging_expanded_endpoint(mocked_func): log_claim_stats_v2(test_claim, classifier_response, test_expanded_request) mocked_func.assert_called_with(expected_claim_log) assert mocked_func.call_count == 3 + + +@patch("src.python_src.api.log_as_json") +def test_claim_testing_log(mocked_func): + """ + Tests that the testing claim flag is logged correctly + """ + test_contention = Contention( + contention_text="acl tear right", + contention_type="NEW", + ) + test_claim = VaGovClaim( + claim_id=100, + form526_submission_id=500, + contentions=[test_contention], + testing_claim=True, + ) + classified_contention = ClassifiedContention( + classification_code=8997, + classification_name="Musculoskeletal - Knee", + diagnostic_code=None, + contention_type="NEW", + ) + log_contention_stats( + test_contention, classified_contention, test_claim, test_expanded_request + ) + + test_response = ClassifierResponse( + contentions=[classified_contention], + claim_id=test_claim.claim_id, + form526_submission_id=test_claim.form526_submission_id, + is_fully_classified=False, + num_processed_contentions=1, + num_classified_contentions=1, + ) + + expected_logging_dict = { + "vagov_claim_id": test_claim.claim_id, + "claim_type": "new", + "classification_code": classified_contention.classification_code, + "classification_name": classified_contention.classification_name, + "contention_text": "unmapped contention text ['acl tear']", + "diagnostic_code": "None", + "is_in_dropdown": False, + "is_lookup_table_match": True, + "is_multi_contention": False, + "endpoint": "/expanded-contention-classification", + "testing_claim": True, + "processed_contention_text": "acl tear", + } + + mocked_func.assert_called_with(expected_logging_dict) + + log_claim_stats_v2(test_claim, test_response, test_expanded_request) + + expected_claim_logging_dict = { + "claim_id": test_claim.claim_id, + "form526_submission_id": test_claim.form526_submission_id, + "is_fully_classified": test_response.is_fully_classified, + "percent_clasified": ( + test_response.num_classified_contentions + / test_response.num_processed_contentions + ) + * 100, + "num_processed_contentions": test_response.num_processed_contentions, + "num_classified_contentions": test_response.num_classified_contentions, + "endpoint": "/expanded-contention-classification", + "testing_claim": True, + } + + mocked_func.assert_called_with(expected_claim_logging_dict) + assert mocked_func.call_count == 2 From 58ca15b6a96cab607435ae054a320640ed5f66ca Mon Sep 17 00:00:00 2001 From: Tyler Spangler <142833783+tyler-spangler6@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:11:43 -0700 Subject: [PATCH 3/4] renamed api parameter to be more descriptive --- domain-cc/cc-app/src/python_src/api.py | 4 +-- .../cc-app/src/python_src/pydantic_models.py | 2 +- domain-cc/cc-app/tests/test_logging.py | 28 +++++++++---------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/domain-cc/cc-app/src/python_src/api.py b/domain-cc/cc-app/src/python_src/api.py index 07a80e7ad..da849772e 100644 --- a/domain-cc/cc-app/src/python_src/api.py +++ b/domain-cc/cc-app/src/python_src/api.py @@ -143,7 +143,7 @@ def log_contention_stats( "is_lookup_table_match": classification_code is not None, "is_multi_contention": is_multi_contention, "endpoint": request.url.path, - "testing_claim": claim.testing_claim, + "is_ab_test_claim": claim.is_ab_test_claim, } if request.url.path == "/expanded-contention-classification": @@ -175,7 +175,7 @@ def log_claim_stats_v2( "num_processed_contentions": response.num_processed_contentions, "num_classified_contentions": response.num_classified_contentions, "endpoint": request.url.path, - "testing_claim": claim.testing_claim, + "is_ab_test_claim": claim.is_ab_test_claim, } ) diff --git a/domain-cc/cc-app/src/python_src/pydantic_models.py b/domain-cc/cc-app/src/python_src/pydantic_models.py index c765f0736..2a6f6edab 100644 --- a/domain-cc/cc-app/src/python_src/pydantic_models.py +++ b/domain-cc/cc-app/src/python_src/pydantic_models.py @@ -37,7 +37,7 @@ class VaGovClaim(BaseModel): claim_id: int form526_submission_id: int contentions: conlist(Contention, min_length=1) - testing_claim: Optional[bool] = False + is_ab_test_claim: Optional[bool] = False class ClassifiedContention(BaseModel): diff --git a/domain-cc/cc-app/tests/test_logging.py b/domain-cc/cc-app/tests/test_logging.py index df032da40..d733fba44 100644 --- a/domain-cc/cc-app/tests/test_logging.py +++ b/domain-cc/cc-app/tests/test_logging.py @@ -68,7 +68,7 @@ def test_log_contention_stats_expanded(mocked_func): "is_lookup_table_match": True, "is_multi_contention": False, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": "acl tear", } @@ -108,7 +108,7 @@ def test_non_classified_contentions(mocked_func): "is_lookup_table_match": False, "is_multi_contention": False, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": None, } mocked_func.assert_called_once_with(expected_log) @@ -159,7 +159,7 @@ def test_multiple_contentions(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": "tinnitus ringing hissing ears", }, { @@ -173,7 +173,7 @@ def test_multiple_contentions(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": "anxiety", }, ] @@ -234,7 +234,7 @@ def test_contentions_with_pii(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": None, }, { @@ -248,7 +248,7 @@ def test_contentions_with_pii(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": None, }, ] @@ -311,7 +311,7 @@ def test_log_claim_stats(mocked_func): "num_processed_contentions": 2, "num_classified_contentions": 1, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, } log_claim_stats_v2(test_claim, classifier_response, test_expanded_request) mocked_func.assert_called_once_with(expected_log) @@ -353,7 +353,7 @@ def test_current_classifier_contention(mocked_func): "is_lookup_table_match": False, "is_multi_contention": False, "endpoint": "/va-gov-claim-classifier", - "testing_claim": False, + "is_ab_test_claim": False, } mocked_func.assert_called_once_with(expected_logging_dict) @@ -404,7 +404,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "is_lookup_table_match": False, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": None, }, { @@ -418,7 +418,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "is_lookup_table_match": True, "is_multi_contention": True, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, "processed_contention_text": "anxiety", }, ] @@ -439,7 +439,7 @@ def test_full_logging_expanded_endpoint(mocked_func): "num_processed_contentions": 2, "num_classified_contentions": 1, "endpoint": "/expanded-contention-classification", - "testing_claim": False, + "is_ab_test_claim": False, } for i in range(len(test_contentions)): @@ -469,7 +469,7 @@ def test_claim_testing_log(mocked_func): claim_id=100, form526_submission_id=500, contentions=[test_contention], - testing_claim=True, + is_ab_test_claim=True, ) classified_contention = ClassifiedContention( classification_code=8997, @@ -501,7 +501,7 @@ def test_claim_testing_log(mocked_func): "is_lookup_table_match": True, "is_multi_contention": False, "endpoint": "/expanded-contention-classification", - "testing_claim": True, + "is_ab_test_claim": True, "processed_contention_text": "acl tear", } @@ -521,7 +521,7 @@ def test_claim_testing_log(mocked_func): "num_processed_contentions": test_response.num_processed_contentions, "num_classified_contentions": test_response.num_classified_contentions, "endpoint": "/expanded-contention-classification", - "testing_claim": True, + "is_ab_test_claim": True, } mocked_func.assert_called_with(expected_claim_logging_dict) From 7645d2d3ab07200dee150162dc532b7183fa0958 Mon Sep 17 00:00:00 2001 From: Tyler Spangler <142833783+tyler-spangler6@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:19:42 -0700 Subject: [PATCH 4/4] removed unneccesary comment --- domain-cc/cc-app/src/python_src/api.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/domain-cc/cc-app/src/python_src/api.py b/domain-cc/cc-app/src/python_src/api.py index da849772e..6b455b1d1 100644 --- a/domain-cc/cc-app/src/python_src/api.py +++ b/domain-cc/cc-app/src/python_src/api.py @@ -151,8 +151,6 @@ def log_contention_stats( logging_dict, contention.contention_text, log_contention_text ) - # log_as_json(logging_dict) - # else: log_as_json(logging_dict)