Address review: count ranges, empty-candidate guard, CSV newlines, dead code

Author: noderaven

breach_scraper/cli.py

@@ -230,7 +230,6 @@ def scrape_all_sources(
     retries: int = 3,
 ) -> list[dict[str, Any]]:
     combined: list[dict[str, Any]] = []
-    failures: list[str] = []
     for source in _selected_sources(source_keys):
         try:
             LOGGER.info("Fetching source %s", source.key)
@@ -254,14 +253,9 @@ def scrape_all_sources(
             )
             combined.extend(_annotate_record(record, source) for record in filtered_records)
         except Exception as exc:
-            message = f"source {source.key} failed: {exc}"
-            failures.append(message)
-            LOGGER.error(message)
             if strict:
-                raise SourceRunError(message) from exc
-
-    if failures and not combined and strict:
-        raise SourceRunError("; ".join(failures))
+                raise SourceRunError(f"source {source.key} failed: {exc}") from exc
+            LOGGER.error("source %s failed: %s", source.key, exc)
 
     return _sort_records(combined)
 

breach_scraper/http.py

@@ -41,6 +41,9 @@ def fetch_url(
     (capped at 30s). HTTP 403 raises an actionable error; other 4xx are not
     retried and fall through to the next candidate.
     """
+    if not candidates:
+        raise ValueError("At least one candidate URL must be provided.")
+
     request_headers = {"User-Agent": user_agent or DEFAULT_USER_AGENT, **DEFAULT_HEADERS}
     if headers:
         request_headers.update(headers)

breach_scraper/output.py

@@ -102,7 +102,7 @@ def _to_csv(records: list[dict[str, Any]]) -> str:
             if key not in columns:
                 columns.append(key)
     buffer = StringIO()
-    writer = csv.DictWriter(buffer, fieldnames=columns)
+    writer = csv.DictWriter(buffer, fieldnames=columns, lineterminator="\n")
     writer.writeheader()
     writer.writerows(records)
     return buffer.getvalue()

breach_scraper/sources/wa_atg.py

@@ -48,10 +48,10 @@ def _normalize_count(value: str) -> str:
     cleaned = _clean_text(value)
     if not cleaned:
         return ""
-    digits = re.sub(r"[^\d]", "", cleaned)
-    if not digits:
-        return cleaned
-    return f"{int(digits):,}"
+    no_commas = cleaned.replace(",", "")
+    if no_commas.isdigit():
+        return f"{int(no_commas):,}"
+    return cleaned
 
 
 def _field_sort_key(item: tuple[str, str]) -> tuple[int, int | str]:

tests/test_cli.py

@@ -60,6 +60,12 @@ def test_default_window_filters_old_records(self) -> None:
         self.assertEqual(rc, 0)
         self.assertEqual(json.loads(out), [])
 
+    def test_offline_csv_has_unix_line_endings(self) -> None:
+        rc, out, _ = run(["--input-html", str(FIXTURE), "--output", "csv", *RANGE])
+        self.assertEqual(rc, 0)
+        self.assertNotIn("\r", out)
+        self.assertIn("date_reported,organization_name", out)
+
 
 class TestCliOnline(unittest.TestCase):
     @mock.patch("breach_scraper.http.urlopen")

tests/test_http.py

@@ -87,6 +87,10 @@ def test_candidate_fallback(self, mock_urlopen: mock.Mock, _sleep: mock.Mock) ->
         self.assertEqual(result, "ok")
         self.assertEqual(mock_urlopen.call_count, 2)
 
+    def test_empty_candidates_raises_value_error(self) -> None:
+        with self.assertRaises(ValueError):
+            fetch_url([])
+
 
 if __name__ == "__main__":
     unittest.main()

tests/test_wa_atg.py

@@ -28,6 +28,10 @@ def test_normalize_record_dates_and_counts(self) -> None:
         self.assertEqual(out["date_reported"], "2024-06-01")
         self.assertEqual(out["number_of_washingtonians_affected"], "1,234")
 
+    def test_normalize_count_keeps_ranges_intact(self) -> None:
+        out = normalize_record({"number_of_washingtonians_affected": "1,200 - 1,500"})
+        self.assertEqual(out["number_of_washingtonians_affected"], "1,200 - 1,500")
+
 
 if __name__ == "__main__":
     unittest.main()