Skip to content

Fix mask_amount_rounded type mismatch and add missing PII maskers to financial services#22

Open
puichinglee wants to merge 1 commit into
mainfrom
pcl-fix-financial-masking
Open

Fix mask_amount_rounded type mismatch and add missing PII maskers to financial services#22
puichinglee wants to merge 1 commit into
mainfrom
pcl-fix-financial-masking

Conversation

@puichinglee
Copy link
Copy Markdown
Contributor

@puichinglee puichinglee commented Apr 21, 2026

Summary

  • Fix mask_amount_rounded runtime breakage: mask_amount_round returned STRING with a $ prefix (CONCAT('$', CAST(ROUND(...)))) but the underlying columns are DECIMAL/DOUBLE, causing type errors at query time. Renamed to mask_amount_rounded to match the canonical name in function_registry.json and changed the return type to RETURNS DECIMAL(18,2) with a clean ROUND(amount, -3) body.
  • Add missing PII maskers: The LLM already tags Customer Email columns and routes them to mask_email, but the function definition didn't exist in financial_services.yaml. Added mask_email, mask_phone, and mask_address (ported from retail.yaml) plus Phone Number and Customer Address identifiers so all tagged PII columns have matching functions.
  • Updated prompt_overlay to list the new functions and identifiers.

… PII maskers

- Rename mask_amount_round → mask_amount_rounded (canonical name in function_registry.json)
- Fix return type from RETURNS STRING to RETURNS DECIMAL(18,2) and remove $ prefix that broke DECIMAL/DOUBLE columns at runtime
- Add mask_email, mask_phone, mask_address functions (ported from retail.yaml) so the LLM has matching functions for tagged PII columns
- Add Phone Number and Customer Address identifiers
- Update prompt_overlay to reflect all changes

Co-authored-by: Isaac
@puichinglee puichinglee requested a review from louiscsq April 21, 2026 03:51
@puichinglee
Copy link
Copy Markdown
Contributor Author

puichinglee commented Apr 21, 2026

Test Results: Unit Tests

tests/test_industry_overlays.py::TestRetailContent::test_uses_hashing_for_names PASSED [ 77%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_loads_single_industry PASSED [ 78%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_loads_multiple_industries PASSED [ 78%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_case_insensitive PASSED [ 79%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_missing_industry_exits PASSED [ 80%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_returns_empty_for_empty_list PASSED [ 81%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_includes_group_templates PASSED [ 82%]
tests/test_industry_overlays.py::TestLoadIndustryOverlays::test_includes_access_patterns PASSED [ 83%]
tests/test_industry_overlays.py::TestBuildPromptWithIndustries::test_no_industries_produces_base_prompt PASSED [ 84%]
tests/test_industry_overlays.py::TestBuildPromptWithIndustries::test_industry_overlay_injected_before_my_tables PASSED [ 84%]
tests/test_industry_overlays.py::TestBuildPromptWithIndustries::test_multiple_industries_all_injected PASSED [ 85%]
tests/test_industry_overlays.py::TestBuildPromptWithIndustries::test_industry_with_mode_governance PASSED [ 86%]
tests/test_industry_overlays.py::TestBuildPromptWithIndustries::test_none_industries_no_overlay PASSED [ 87%]
tests/test_industry_overlays.py::TestBuildPromptWithBothOverlays::test_both_overlays_present PASSED [ 88%]
tests/test_industry_overlays.py::TestBuildPromptWithBothOverlays::test_country_appears_before_industry PASSED [ 89%]
tests/test_industry_overlays.py::TestBuildPromptWithBothOverlays::test_both_before_my_tables PASSED [ 89%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_loads_financial_hints PASSED [ 90%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_loads_healthcare_hints PASSED [ 91%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_loads_retail_hints PASSED [ 92%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_loads_function_categories PASSED [ 93%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_multi_industry_merges PASSED [ 94%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_missing_industry_skipped PASSED [ 94%]
tests/test_industry_overlays.py::TestLoadIndustryCategories::test_empty_list_returns_empty PASSED [ 95%]
tests/test_industry_overlays.py::TestInferColumnCategoriesWithIndustry::test_account_number_detected PASSED [ 96%]
tests/test_industry_overlays.py::TestInferColumnCategoriesWithIndustry::test_patient_id_detected PASSED [ 97%]
tests/test_industry_overlays.py::TestInferColumnCategoriesWithIndustry::test_loyalty_number_detected PASSED [ 98%]
tests/test_industry_overlays.py::TestInferColumnCategoriesWithIndustry::test_existing_us_patterns_still_work PASSED [ 99%]
tests/test_industry_overlays.py::TestFunctionExpectedCategoriesExtensionIndustry::test_industry_categories_can_be_merged PASSED [100%]

============================= 119 passed in 0.75s ==============================

Coverage

  • test_validate_abac.py (44 tests) — all pass: validates ABAC config structure, tag policies, FGAC policies, SQL function parsing, condition matching, ACL groups
  • test_industry_overlays.py (75 tests) — all pass: validates YAML integrity, identifier/function cross-refs, prompt overlay injection, category inference, function category merging

Key validations for this PR

  • test_masking_functions_referenced_by_identifiers_exist[financial_services] — confirms all identifiers (including new Phone Number, Customer Address) reference functions that exist in the YAML
  • test_masking_functions_have_required_fields[financial_services] — confirms mask_amount_rounded, mask_email, mask_phone, mask_address all have valid name/signature/body
  • test_loads_function_categories — confirms industry overlay loading correctly maps functions to categories

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