Fix/146 pii scrubber phone number

Journal.md

@@ -0,0 +1,131 @@
+## Week 7 — Issue selection
+
+**Issue link:** https://github.com/ascherj/pathreview/issues/146#
+
+**Issue title:** PII scrubber fails to redact parenthesized US phone numbers
+
+**Tier:** Tier 1
+
+**Problem summary:**
+The personal information scrubbery in pii_scrubber.py is not redacting phone numbers with a parenthesized beginning format. The scrub is not redacting and the format is not being detected. A successful fix would lead to the parenthesized format of phone numbers being detected and redacted from text. This file is in the safety folder, but I cannot find reference to it outside the test suite. So I assume it is not integrated into the project and not affecting other files. 
+
+**Branch name:** fix/146-pii-scrubber-phone-number
+
+**Setup confirmation:** Yes, app runs locally at localhost:5173.
+
+**Cohort ledger:** Yes, issue was added to cohort ledger.
+
+## "Is this right for me?" Checklist:
+### Tier Fit
+- Is this an appropriate tier for my experience? Yes, because I have not done open source contributions, and I have fixed similar bugs before.
+
+### Codebase Readiness
+- **Relevant function/module found:** pii_scrubber.py
+- **Rough implementation plan:**
+  1. Run relevant tests to replicate the bug.
+  2. Read the file.
+  3. Identify the code likely responsible.
+  4. Propose the fix.
+  5. Test the fix
+  6. Run entire test suite. 
+  7. Repeat if issue is not reolved.
+
+- **Relevant test files:**
+- test_us_phone_number_redaction, test_us_phone_formats, test_detect_phone_pii, test_phone_at_start_of_text in tests/unit/test_pii_scrubber.py
+
+### Scope & Time
+- **Others already working on it?** When I first went to claim it no, but now there are many others working on it.
+- **Estimated time:** 2-3 hrs
+- **Can I finish before the deadline?** Yes, seems like a minor issue. I have solved similar bugs before.
+- **Dependencies/blockers:** None
+
+## Week 8 — Reproduction & solution planning
+
+**Reproduction commit link:** [link to commit documenting the reproduced issue](https://github.com/a-maryam/pathreview/commit/7ac52cff2e60e3c8cf7efe38fd7847ba22a823be)
+
+**Reproduction summary:**
+I reproduced the issue by following the instructions for the bug on github: 
+
+I ran the following script in the project root:
+```
+# script to reproduce pii bug
+import os
+import sys
+
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+
+from pii_scrubber import PIIScrubber
+s = PIIScrubber()
+print(s.scrub('Call me at (555) 123-4567 or 555-123-4567'))
+# observed: 'Call me at (555) 123-4567 or [REDACTED]'
+print(s.detect('Call me at (555) 123-4567'))
+# observed: []
+```
+**PLAN.md link:** [PLAN.md](PLAN.md)
+
+**Walkthrough video (recommended):** [link to your Loom video, ≤2 min — recommended, not graded]
+
+**Blockers or open questions:**
+[Anything you're still uncertain about going into Week 9, or leave blank]
+
+## Week 9 — Solution building & PR submission
+
+### Check-in 1 (mid-week)
+
+**Current progress:**
+Completed all steps. Came up with a fix, applied code fix, wrote tests, ran full test suite. 
+
+**Next steps:**
+Check formatting and make all necessary commits/journals.
+
+**Blockers:**
+
+---
+
+### Check-in 2 (end of week)
+
+**PR link:** [link to submitted pull request\](https://github.com/ascherj/pathreview/pull/1017)
+
+**Branch:** fix/146-pii-scrubber-phone-number
+
+**What you built:**
+I fixed the regex, so that a space counts as a separator and the matching doesn't break after the ). Also put \b before the first digit group so that the position there is a boundary between a word character and a non-word character. Fixes the regex of the redactor to catch phone number formats of the type (555) 774-3242
+
+**Tests added or updated:**
+test_pii_scrubber.py. Added tests to check that phone numbers of the type (555) 123-4567 are redacted. Tested (555)-123-4567, a similar type. Added test to check that similar pattern not meeting phone number length was not matched. Tested detect() to make sure that it was catching the full phone number. 
+
+**Self-review confirmation:** [X] make check passes  [X] make test-unit passes
+
+**Draft PR feedback received from:** [none]
+
+## Week 10 — Iteration & reflection
+
+### Reviewer feedback
+
+**Feedback received:** [ ] Yes  [X] No — still awaiting review
+
+**Summary of feedback:**
+No review came in because I had to get an extension. 
+
+**How you responded:**
+[What changes did you make, or what did you reply? If no feedback,
+leave blank.]
+
+---
+
+### Reflection
+
+**What was harder than you expected?**
+I think the pre-commit hooks kind of annoyed me. Also, just understanding regex is pretty tough. And I guess recently having the mental bandwidth for this stuff on top of everything else was difficult. 
+
+**What did you learn about working in a large codebase?**
+It's messy and it can be hard to know what is going on. It takes research and time to understand. 
+
+**How did AI tools help — and where did they fall short?**
+I had to go read about regex to understand it. And I guess they didn't check what the linting would, so I fixed that. 
+
+**What would you do differently if you started over?**
+Spend more time on the implementation. I rushed a bit just because I had other things to tend to. 
+
+**What are you most proud of from this module?**
+I am mainly proud of fixing the git mistakes that I made. I know it is useful to know these tidbits of git. 

PLAN.md

@@ -0,0 +1,43 @@
+## Solution plan
+
+**Issue:** [PII scrubber fails to redact parenthesized US phone numbers
+](https://github.com/ascherj/pathreview/issues/146#)
+
+**Link:** https://github.com/ascherj/pathreview/issues/146#
+
+### Understand
+What is the root cause of this issue? What behavior is expected vs. actual? Line 15 in pii_scrubber.py is the issue because a space between the parentheses breaks the pattern matching of the numbers that follow, 555) will actually be catched, but it breaks down on the rest: 123-34567/
+
+### Map
+Which files, functions, or modules are involved?
+List the specific files you expect to touch. I expect to edit the regex in pii_scrubber.py.
+
+### Plan
+What are the steps to fix this issue?
+Break it into 3–5 concrete sub-tasks.
+1. Read the pii_scrubber.py code to try to understand the issue.
+2. Research or use AI assistance if needed.
+3. Write a fix (I think just editing the regex on line 15 of pii_scrubber.py will be the fix)
+4. Try failing tests and write new tests. Make sure detect function and scrub function are doing their job.
+5. Make sure all tests, integration and unit pass.
+
+### Inputs & outputs
+What does your fix take as input? What should it produce or change?
+The input is text with phone numbers of the format (555) 123-4567. It should product [REDACTED]
+```
+'Call me at (555) 123-4567 or 555-123-4567'
+
+Should produce:
+
+'Call me at [REDACTED] or [REDACTED]'
+```
+
+And s.detect() should detect the number in it with the regex.
+
+### Risks & unknowns
+What could go wrong? What are you still unsure about?
+If you adjust the regex incorrectly, you could redact the wrong info. If for some reason, there are lists of numbers like (777) 345 they could get redacted. That seems like an uncommon scenario. I guess I am a little bit unsure about regex; I don't have a deep understanding of it, so I will have to read up.
+
+### Edge cases
+What inputs or states should your fix handle gracefully?
+(777) 345 might get redacted. We need to make sure numbers in parentheses aren't redacted by accident. (555)-123-4567 doesn't work correctly right no either, this is what happens: ([REDACTED].

safety/pii_scrubber.py

@@ -1,6 +1,7 @@
 """PII detection and scrubbing."""
 
 import re
+
 import structlog
 
 logger = structlog.get_logger()
@@ -12,10 +13,13 @@ class PIIScrubber:
     # Regex patterns for common PII
     PII_PATTERNS = {
         "email": r"\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b",
-        "phone_us": r"\b(?:\+?1[-.]?)?\(?([0-9]{3})\)?[-.]?([0-9]{3})[-.]?([0-9]{4})\b",
+        "phone_us": r"(?:\+?1[-.\s]?)?\(?\b([0-9]{3})\)?[-.\s]?([0-9]{3})[-.\s]?([0-9]{4})\b",
         "phone_intl": r"\+[0-9]{1,3}[-.]?[0-9]{1,14}",
         "ssn": r"\b(?!000|666)[0-9]{3}-(?!00)[0-9]{2}-(?!0000)[0-9]{4}\b",
-        "street_address": r"\b\d+\s+[A-Za-z\s]+(?:Street|St|Avenue|Ave|Road|Rd|Boulevard|Blvd|Drive|Dr|Lane|Ln|Court|Ct|Circle|Cir|Park|Pl|Plaza|Place|Drive|Dr|Way|Parkway|Pkwy|Point|Pt|Pike|Run|Summit|Summit|Terrace|Ter|Trail|Trl|Tunnel|Turnpike|View|Vista|Vlg|Village|Vly|Valley)",
+        "street_address": r"\b\d+\s+[A-Za-z\s]+(?:Street|St|Avenue|Ave|Road|Rd|Boulevard|Blvd|"
+        "DriveDr|Lane|Ln|Court|Ct|Circle|Cir|Park|Pl|Plaza|Place|Drive|Dr|Way|Parkway|Pkwy"
+        "|Point|Pt|Pike|Run|Summit|Summit|Terrace|Ter|Trail|Trl|Tunnel|Turnpike"
+        "|View|Vista|Vlg|Village|Vly|Valley)",
     }
 
     def scrub(self, text: str) -> str:
@@ -29,7 +33,7 @@ def scrub(self, text: str) -> str:
         """
         scrubbed = text
 
-        for pii_type, pattern in self.PII_PATTERNS.items():
+        for _pii_type, pattern in self.PII_PATTERNS.items():
             scrubbed = re.sub(pattern, "[REDACTED]", scrubbed, flags=re.IGNORECASE)
 
         return scrubbed
@@ -47,13 +51,17 @@ def detect(self, text: str) -> list[dict]:
 
         for pii_type, pattern in self.PII_PATTERNS.items():
             for match in re.finditer(pattern, text, flags=re.IGNORECASE):
-                detected.append({
-                    "type": pii_type,
-                    "value": match.group(),
-                    "start": match.start(),
-                    "end": match.end()
-                })
-
-        logger.info("pii_detected", count=len(detected), types=len(set(d["type"] for d in detected)))
+                detected.append(
+                    {
+                        "type": pii_type,
+                        "value": match.group(),
+                        "start": match.start(),
+                        "end": match.end(),
+                    }
+                )
+
+        logger.info(
+            "pii_detected", count=len(detected), types=len(set(d["type"] for d in detected))
+        )
 
         return detected

tests/unit/test_pii_scrubber.py

@@ -53,6 +53,41 @@ def test_us_phone_formats(self, scrubber):
             scrubbed = scrubber.scrub(text)
             assert "[REDACTED]" in scrubbed
 
+    def test_parenthesized_phone_with_space_redacted(self, scrubber):
+        """Test parenthesized phone number followed by a space is fully redacted."""
+        text = "Call me at (555) 123-4567 or 555-123-4567"
+        scrubbed = scrubber.scrub(text)
+
+        assert "[REDACTED]" in scrubbed
+        assert "(" not in scrubbed
+        assert "555" not in scrubbed
+
+    def test_parenthesized_phone_with_dash_after_paren(self, scrubber):
+        """Test parenthesized phone number with a dash right after the closing paren."""
+        text = "Call me at (555)-123-4567"
+        scrubbed = scrubber.scrub(text)
+
+        assert "[REDACTED]" in scrubbed
+        assert "(" not in scrubbed
+        assert "555" not in scrubbed
+
+    def test_detect_parenthesized_phone_with_space(self, scrubber):
+        """Test detect() finds the full parenthesized phone number, including the paren."""
+        text = "Call me at (555) 123-4567"
+        detected = scrubber.detect(text)
+
+        phone_detections = [d for d in detected if d["type"] == "phone_us"]
+        assert len(phone_detections) == 1
+        assert phone_detections[0]["value"] == "(555) 123-4567"
+
+    def test_short_parenthesized_number_not_flagged_as_phone(self, scrubber):
+        """Test a short parenthesized number that isn't a full phone number isn't flagged."""
+        text = "Room (777) 345"
+        detected = scrubber.detect(text)
+
+        phone_detections = [d for d in detected if d["type"] == "phone_us"]
+        assert len(phone_detections) == 0
+
     def test_international_phone_redaction(self, scrubber):
         """Test international phone number is redacted."""
         text = "Reach me at +44 20 7946 0958"