Add four breach sources: California, Oregon, Maine, HHS OCR#5
Conversation
There was a problem hiding this comment.
Code Review
This pull request registers and implements four new breach scraper sources: California Attorney General (ca_oag), Oregon Department of Justice (or_doj), Maine Attorney General (maine_ag), and HHS OCR Breach Portal (hhs_ocr), along with corresponding unit tests and CLI updates. The code review identifies several critical improvements to make the scrapers more robust: wrapping date parsing in try-except blocks to prevent crashes on malformed dates in the Oregon and HHS scrapers, propagating nested href links in the Maine parser, using a more reliable CSV detection method and ensuring custom User-Agents are passed during POST requests in the HHS scraper, and robustly splitting date ranges in the Oregon scraper using regular expressions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _normalize_single_date(value: str) -> str: | ||
| cleaned = _clean_text(value) | ||
| if not cleaned or cleaned in SENTINEL_DATES: | ||
| return "" | ||
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() |
There was a problem hiding this comment.
Parsing dates with datetime.strptime without a try-except block is extremely fragile. Any malformed date or unexpected format on the Oregon DOJ website will raise a ValueError and crash the entire scraper. Wrap the parsing in a try-except block and return the original string as a fallback.
| def _normalize_single_date(value: str) -> str: | |
| cleaned = _clean_text(value) | |
| if not cleaned or cleaned in SENTINEL_DATES: | |
| return "" | |
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() | |
| def _normalize_single_date(value: str) -> str: | |
| cleaned = _clean_text(value) | |
| if not cleaned or cleaned in SENTINEL_DATES: | |
| return "" | |
| try: | |
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() | |
| except ValueError: | |
| return cleaned |
There was a problem hiding this comment.
Fixed. _normalize_single_date now wraps strptime in try/except and returns the original string on ValueError. Covered by test_invalid_date_returns_original.
| def _normalize_date(value: str) -> str: | ||
| cleaned = _clean_text(value) | ||
| if not cleaned: | ||
| return "" | ||
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() |
There was a problem hiding this comment.
Parsing dates with datetime.strptime without a try-except block is fragile. Any malformed date or unexpected format on the HHS OCR portal will raise a ValueError and crash the entire scraper. Wrap the parsing in a try-except block and return the original string as a fallback.
| def _normalize_date(value: str) -> str: | |
| cleaned = _clean_text(value) | |
| if not cleaned: | |
| return "" | |
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() | |
| def _normalize_date(value: str) -> str: | |
| cleaned = _clean_text(value) | |
| if not cleaned: | |
| return "" | |
| try: | |
| return datetime.strptime(cleaned, "%m/%d/%Y").date().isoformat() | |
| except ValueError: | |
| return cleaned |
There was a problem hiding this comment.
Fixed. _normalize_date now wraps strptime in try/except and returns the original string on ValueError.
| if tag == "li" and self.item_stack: | ||
| item = self.item_stack.pop() | ||
| text = item.text | ||
| if self.item_stack: | ||
| self.item_stack[-1].add_child_text(text) | ||
| elif text: | ||
| self.items.append((self.current_section, text, item.href)) |
There was a problem hiding this comment.
When parsing nested list items (e.g., <li> inside another <li>), the nested item's href is set on the nested _DetailItem. However, when the nested item is popped and its text is added to the parent via add_child_text, its href is completely lost. Propagate the href to the parent item if the parent doesn't already have one.
| if tag == "li" and self.item_stack: | |
| item = self.item_stack.pop() | |
| text = item.text | |
| if self.item_stack: | |
| self.item_stack[-1].add_child_text(text) | |
| elif text: | |
| self.items.append((self.current_section, text, item.href)) | |
| if tag == "li" and self.item_stack: | |
| item = self.item_stack.pop() | |
| text = item.text | |
| if self.item_stack: | |
| self.item_stack[-1].add_child_text(text) | |
| if item.href and not self.item_stack[-1].href: | |
| self.item_stack[-1].href = item.href | |
| elif text: | |
| self.items.append((self.current_section, text, item.href)) |
There was a problem hiding this comment.
Fixed. Nested
| def _parse_csv_export(text: str) -> list[dict[str, str]]: | ||
| if not text.lstrip().startswith('"'): | ||
| return [] |
There was a problem hiding this comment.
Checking if the text starts with a double quote (") to detect a CSV export is fragile. If the first field is unquoted or if there are leading spaces, this check will fail. A more robust approach is to verify that the content does not start with < (which indicates HTML) and contains expected CSV headers like "Covered Entity Type".
| def _parse_csv_export(text: str) -> list[dict[str, str]]: | |
| if not text.lstrip().startswith('"'): | |
| return [] | |
| def _parse_csv_export(text: str) -> list[dict[str, str]]: | |
| cleaned = text.lstrip() | |
| if cleaned.startswith("<") or "Covered Entity Type" not in cleaned: | |
| return [] |
There was a problem hiding this comment.
Fixed. _parse_csv_export now rejects content starting with '<' and requires the 'Covered Entity Type' header. Covered by test_html_is_not_treated_as_csv and test_csv_without_known_header_is_rejected.
| export_request = Request( | ||
| url, | ||
| data=data, | ||
| headers={**_build_headers(), "Content-Type": "application/x-www-form-urlencoded"}, | ||
| ) |
There was a problem hiding this comment.
The POST request to export the CSV does not pass the custom user_agent to _build_headers(), meaning it will fall back to the default Chrome 123 User-Agent. This causes an inconsistency with the initial GET request if a custom User-Agent was configured.
| export_request = Request( | |
| url, | |
| data=data, | |
| headers={**_build_headers(), "Content-Type": "application/x-www-form-urlencoded"}, | |
| ) | |
| export_request = Request( | |
| url, | |
| data=data, | |
| headers={**_build_headers(user_agent), "Content-Type": "application/x-www-form-urlencoded"}, | |
| ) |
There was a problem hiding this comment.
Fixed. The export POST now passes _build_headers(user_agent) so a custom --user-agent is used consistently.
| if " - " in part: | ||
| range_parts = [_normalize_single_date(item) for item in part.split(" - ")] | ||
| range_parts = [item for item in range_parts if item] | ||
| if range_parts: | ||
| normalized_parts.append(" to ".join(range_parts)) | ||
| continue |
There was a problem hiding this comment.
Splitting date ranges by strictly checking for " - " (with spaces) is fragile. If the website contains a date range with different spacing (e.g., "8/13/2025-8/14/2025"), it won't be split correctly. Use re.split(r'\s*-\s*', part) to robustly handle any spacing around the hyphen.
| if " - " in part: | |
| range_parts = [_normalize_single_date(item) for item in part.split(" - ")] | |
| range_parts = [item for item in range_parts if item] | |
| if range_parts: | |
| normalized_parts.append(" to ".join(range_parts)) | |
| continue | |
| if "-" in part: | |
| range_parts = [_normalize_single_date(item) for item in re.split(r'\s*-\s*', part)] | |
| range_parts = [item for item in range_parts if item] | |
| if range_parts: | |
| normalized_parts.append(" to ".join(range_parts)) | |
| continue |
There was a problem hiding this comment.
Fixed. Ranges are now split with re.split(r'\s*-\s*', part), guarded by 'if "-" in part'. Covered by test_tight_range_is_split.
…Maine nested links
Stage 2 of the multi-source integration. Adds four sources behind the registry from stage 1 (stdlib-only).
Summary
ca_oag(California OAG),or_doj(Oregon DOJ),maine_ag(Maine AG),hhs_ocr(HHS OCR breach portal), each exposingDEFAULT_URL/fetch_html/parse_breach_tablewith source-specific normalization.http.fetch_url(retries, backoff, 403 handling); HHS keeps its cookie-jar two-step CSV export (it needs session cookies).registry.py;breach-scraper --list-sourcesnow shows five sources and a run aggregates across them with date filtering.E501ignore for the long inline HTML/CSV fixtures in tests.Test Plan
breach-scraper --list-sourceslists wa_atg, ca_oag, or_doj, maine_ag, hhs_ocrNext stage:
reportsummary + the HTML web UI.