diff --git a/src/nhs_scraper/pipeline/extract.py b/src/nhs_scraper/pipeline/extract.py index 6d3cb93..4ad4a3b 100644 --- a/src/nhs_scraper/pipeline/extract.py +++ b/src/nhs_scraper/pipeline/extract.py @@ -4,14 +4,12 @@ returns validated ``WaitingTimeRecord`` objects. Behaviour is pinned by the characterisation fixtures and golden dataset. -Expected page shape (My Planned Care trust page): - -- ``h1`` — provider (trust) name -- ``section.specialty`` blocks, each with an ``h3`` specialty name -- within a section, ``h4`` headings ("First Outpatient Appointment" / - "Treatment") each followed by a table whose data row holds - "N weeks" values for average and 8-in-10 waits -- footer text "Page last updated: DD/MM/YYYY" +Two layouts are supported. The legacy layout (pre-2026, kept as the +characterisation baseline) uses ``section.specialty`` blocks with ``h4`` +metric headings. The 2026 layout uses ``div.inner_details_holder`` blocks +with table ```` metric labels and ``n/a`` cells; its footer +reads "This page was last updated on D Month YYYY". New-layout extraction +is tried first; a page with no 2026 holders falls back to legacy. """ from __future__ import annotations @@ -19,18 +17,19 @@ import re from datetime import date, datetime -from bs4 import BeautifulSoup, Tag +from bs4 import BeautifulSoup from nhs_scraper.domain import Metric, Page, WaitingTimeRecord -_METRIC_BY_HEADING = { +_METRIC_BY_LABEL = { "first outpatient appointment": Metric.FIRST_OUTPATIENT_APPOINTMENT, "treatment": Metric.TREATMENT, } _WEEKS_PATTERN = re.compile(r"(\d+)\s*weeks?", re.IGNORECASE) -_LAST_UPDATED_PATTERN = re.compile( - r"page last updated:\s*(\d{2}/\d{2}/\d{4})", re.IGNORECASE +_LEGACY_FOOTER = re.compile(r"page last updated:\s*(\d{2}/\d{2}/\d{4})", re.IGNORECASE) +_2026_FOOTER = re.compile( + r"this page was last updated on\s+(\d{1,2}\s+\w+\s+\d{4})", re.IGNORECASE ) @@ -41,10 +40,12 @@ def _parse_weeks(cell_text: str) -> int | None: def _parse_page_last_updated(soup: BeautifulSoup) -> date | None: - match = _LAST_UPDATED_PATTERN.search(soup.get_text(" ", strip=True)) - if not match: - return None - return datetime.strptime(match.group(1), "%d/%m/%Y").date() + text = soup.get_text(" ", strip=True) + if match := _LEGACY_FOOTER.search(text): + return datetime.strptime(match.group(1), "%d/%m/%Y").date() + if match := _2026_FOOTER.search(text): + return datetime.strptime(match.group(1), "%d %B %Y").date() + return None def _provider_name(soup: BeautifulSoup) -> str | None: @@ -52,31 +53,72 @@ def _provider_name(soup: BeautifulSoup) -> str | None: return heading.get_text(strip=True) if heading else None -def _table_values(table: Tag) -> tuple[int | None, int | None]: - rows = table.find_all("tr") - if len(rows) < 2: - return None, None - cells = [c.get_text(strip=True) for c in rows[1].find_all(["td", "th"])] - average = _parse_weeks(cells[0]) if cells else None - within = _parse_weeks(cells[1]) if len(cells) > 1 else None - return average, within - +def _make_record( + *, region, provider, specialty, source_url, metric, average, within, updated +) -> WaitingTimeRecord: + return WaitingTimeRecord( + region=region, + provider=provider, + specialty=specialty, + source_url=source_url, + metric=metric, + average_wait_weeks=average, + patients_seen_within_weeks=within, + page_last_updated=updated, + ) + + +def _extract_2026( + soup: BeautifulSoup, *, region, provider, source_url, updated +) -> list[WaitingTimeRecord]: + """Extract from the 2026 ``div.inner_details_holder`` layout.""" + records: list[WaitingTimeRecord] = [] + for holder in soup.find_all("div", class_="inner_details_holder"): + heading = holder.find("h3", class_="nhsblue-text0") + if heading is None: + continue + specialty = heading.get_text(strip=True).removesuffix(" - Waiting Times") + + for table in holder.find_all("table", class_="waiting-times-data"): + caption = table.find("caption") + metric = ( + _METRIC_BY_LABEL.get(caption.get_text(strip=True).lower()) + if caption + else None + ) + if metric is None: + continue + average = within = None + for row in table.find_all("tr"): + th, td = row.find("th"), row.find("td") + if th is None or td is None: + continue + label = th.get_text(strip=True).lower() + value = td.get_text(strip=True) + if "average waiting time" in label: + average = _parse_weeks(value) + elif "8 in 10 patients" in label: + within = _parse_weeks(value) + records.append( + _make_record( + region=region, + provider=provider, + specialty=specialty, + source_url=source_url, + metric=metric, + average=average, + within=within, + updated=updated, + ) + ) + return records -def extract_waiting_times(page: Page, *, region: str) -> list[WaitingTimeRecord]: - """Extract every waiting-time record present on a trust ``page``. - Pure: same input, same output, no side effects. Specialties whose data - is unavailable contribute no records; pages without a provider heading - are rejected wholesale. - """ - soup = BeautifulSoup(page.html, "html.parser") - provider = _provider_name(soup) - if provider is None: - return [] - - last_updated = _parse_page_last_updated(soup) +def _extract_legacy( + soup: BeautifulSoup, *, region, provider, source_url, updated +) -> list[WaitingTimeRecord]: + """Extract from the legacy ``section.specialty`` layout.""" records: list[WaitingTimeRecord] = [] - for section in soup.find_all("section", class_="specialty"): specialty_tag = section.find("h3") if specialty_tag is None: @@ -84,22 +126,50 @@ def extract_waiting_times(page: Page, *, region: str) -> list[WaitingTimeRecord] specialty = specialty_tag.get_text(strip=True) for heading in section.find_all("h4"): - metric = _METRIC_BY_HEADING.get(heading.get_text(strip=True).lower()) + metric = _METRIC_BY_LABEL.get(heading.get_text(strip=True).lower()) table = heading.find_next_sibling("table") if metric is None or table is None: continue - average, within = _table_values(table) + rows = table.find_all("tr") + cells = ( + [c.get_text(strip=True) for c in rows[1].find_all(["td", "th"])] + if len(rows) > 1 + else [] + ) + average = _parse_weeks(cells[0]) if cells else None + within = _parse_weeks(cells[1]) if len(cells) > 1 else None records.append( - WaitingTimeRecord( + _make_record( region=region, provider=provider, specialty=specialty, - source_url=page.url, + source_url=source_url, metric=metric, - average_wait_weeks=average, - patients_seen_within_weeks=within, - page_last_updated=last_updated, + average=average, + within=within, + updated=updated, ) ) - return records + + +def extract_waiting_times(page: Page, *, region: str) -> list[WaitingTimeRecord]: + """Extract every waiting-time record present on a trust ``page``. + + Pure: same input, same output, no side effects. Specialties whose data + is unavailable contribute no records; pages without a provider heading + are rejected wholesale. + """ + soup = BeautifulSoup(page.html, "html.parser") + provider = _provider_name(soup) + if provider is None: + return [] + + updated = _parse_page_last_updated(soup) + context = dict( + region=region, + provider=provider, + source_url=page.url, + updated=updated, + ) + return _extract_2026(soup, **context) or _extract_legacy(soup, **context) diff --git a/src/nhs_scraper/pipeline/preflight.py b/src/nhs_scraper/pipeline/preflight.py index e33464f..a516f24 100644 --- a/src/nhs_scraper/pipeline/preflight.py +++ b/src/nhs_scraper/pipeline/preflight.py @@ -1,8 +1,8 @@ """Pre-flight layout probe: detect site structure drift before crawling. Runs the structural signals the extractor depends on against a single -canary page, then the extractor itself end-to-end. Far cheaper and more -diagnosable than discovering an empty CSV after a full crawl. +canary page, then the extractor itself end-to-end. Both the legacy and +the 2026 layouts are recognised; a page matching neither fails. """ from __future__ import annotations @@ -15,9 +15,10 @@ from nhs_scraper.domain import Page from nhs_scraper.pipeline.extract import extract_waiting_times -_KNOWN_METRIC_HEADINGS = {"first outpatient appointment", "treatment"} -_LAST_UPDATED_PATTERN = re.compile( - r"page last updated:\s*\d{2}/\d{2}/\d{4}", re.IGNORECASE +_KNOWN_METRIC_LABELS = {"first outpatient appointment", "treatment"} +_LEGACY_FOOTER = re.compile(r"page last updated:\s*\d{2}/\d{2}/\d{4}", re.IGNORECASE) +_2026_FOOTER = re.compile( + r"this page was last updated on\s+\d{1,2}\s+\w+\s+\d{4}", re.IGNORECASE ) @@ -30,7 +31,7 @@ class LayoutProbeResult: class LayoutDriftError(RuntimeError): - """Raised when the canary page no longer matches the expected layout.""" + """Raised when the canary page no longer matches any known layout.""" def __init__(self, url: str, failures: tuple[str, ...]) -> None: self.url = url @@ -45,29 +46,37 @@ def _structural_failures(page: Page) -> list[str]: if not soup.find("h1"): failures.append("no

provider heading found") - if not soup.find_all("section", class_="specialty"): - failures.append("no
blocks found") - headings = {h.get_text(strip=True).lower() for h in soup.find_all("h4")} - if not headings & _KNOWN_METRIC_HEADINGS: - failures.append("no recognised metric headings (h4) found") + if not ( + soup.find_all("section", class_="specialty") + or soup.find_all("div", class_="inner_details_holder") + ): + failures.append("no recognised specialty blocks found") - has_waiting_table = any( - (row := table.find("tr")) is not None - and "average waiting time" in row.get_text(strip=True).lower() + headings = {h.get_text(strip=True).lower() for h in soup.find_all("h4")} + captions = { + caption.get_text(strip=True).lower() for table in soup.find_all("table") - ) - if not has_waiting_table: + if (caption := table.find("caption")) is not None + } + if not (headings | captions) & _KNOWN_METRIC_LABELS: + failures.append("no recognised metric labels found") + + if not any( + "average waiting time" in th.get_text(strip=True).lower() + for th in soup.find_all("th") + ): failures.append("no waiting-time tables with 'Average waiting time' header found") - if not _LAST_UPDATED_PATTERN.search(soup.get_text(" ", strip=True)): - failures.append("no 'Page last updated: DD/MM/YYYY' footer found") + text = soup.get_text(" ", strip=True) + if not (_LEGACY_FOOTER.search(text) or _2026_FOOTER.search(text)): + failures.append("no recognised last-updated footer found") return failures def probe_layout(page: Page) -> LayoutProbeResult: - """Probe one canary page for the layout the extractor depends on. + """Probe one canary page for a layout the extractor understands. Structural checks run first; only when they all pass is the extractor run end-to-end — a structurally valid page that still yields no diff --git a/tests/fixtures/trust_page_drifted.html b/tests/fixtures/trust_page_drifted.html index 896e901..b475c5b 100644 --- a/tests/fixtures/trust_page_drifted.html +++ b/tests/fixtures/trust_page_drifted.html @@ -1,6 +1,7 @@ - Royal Berkshire Hospital NHS Foundation Trust @@ -24,4 +25,4 @@

Royal Berkshire Hospital NHS Foundation Trust

- + \ No newline at end of file diff --git a/tests/fixtures/trust_page_royal_berkshire.html b/tests/fixtures/trust_page_royal_berkshire.html index c2a4b04..bf6e7af 100644 --- a/tests/fixtures/trust_page_royal_berkshire.html +++ b/tests/fixtures/trust_page_royal_berkshire.html @@ -50,4 +50,4 @@

Paediatric Surgery

- + \ No newline at end of file diff --git a/tests/fixtures/trust_page_royal_berkshire_2026.html b/tests/fixtures/trust_page_royal_berkshire_2026.html new file mode 100644 index 0000000..7b0ee69 --- /dev/null +++ b/tests/fixtures/trust_page_royal_berkshire_2026.html @@ -0,0 +1,82 @@ + + + +Royal Berkshire Hospital NHS Foundation Trust - My Planned Care NHS + +
+
+
+
+

Royal Berkshire Hospital NHS Foundation Trust

+
+
+ +
+
+

Breast - Waiting Times

+ + + + +
First Outpatient Appointment
Average waiting time for first outpatient appointment at this hospital for this specialtyn/a
8 in 10 patients will be seen for a first outpatient appointment at this hospital for this specialty withinn/a
+ + + + +
Treatment
Average waiting time for treatment at this hospital for this specialty4 weeks
8 in 10 patients will be seen for treatment at this hospital for this specialty within7 weeks
+
+
+
+
+
    +
  • The waiting time information is updated each week.
    • +
  • This page was last updated on 7 August 2026.
    • +
+
+
+
+
+ +
+
+

Cardiology - Waiting Times

+ + + + +
First Outpatient Appointment
Average waiting time for first outpatient appointment at this hospital for this specialtyn/a
8 in 10 patients will be seen for a first outpatient appointment at this hospital for this specialty withinn/a
+ + + + +
Treatment
Average waiting time for treatment at this hospital for this specialty8 weeks
8 in 10 patients will be seen for treatment at this hospital for this specialty within14 weeks
+
+
+
+
+
    +
  • The waiting time information is updated each week.
    • +
  • This page was last updated on 7 August 2026.
    • +
+
+
+
+
+ +
+
+

Paediatric Surgery - Waiting Times

+

This information is currently unavailable for this Provider's Specialty. Please check back later.

+
+
+
+
+
+
+ + diff --git a/tests/golden/royal_berkshire_2026_expected.json b/tests/golden/royal_berkshire_2026_expected.json new file mode 100644 index 0000000..863b31e --- /dev/null +++ b/tests/golden/royal_berkshire_2026_expected.json @@ -0,0 +1,42 @@ +[ + { + "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Breast", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "first_outpatient_appointment", + "average_wait_weeks": null, + "patients_seen_within_weeks": null, + "page_last_updated": "2026-08-07" + }, + { + "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Breast", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "treatment", + "average_wait_weeks": 4, + "patients_seen_within_weeks": 7, + "page_last_updated": "2026-08-07" + }, + { + "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Cardiology", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "first_outpatient_appointment", + "average_wait_weeks": null, + "patients_seen_within_weeks": null, + "page_last_updated": "2026-08-07" + }, + { + "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Cardiology", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "treatment", + "average_wait_weeks": 8, + "patients_seen_within_weeks": 14, + "page_last_updated": "2026-08-07" + } +] diff --git a/tests/golden/royal_berkshire_expected.json b/tests/golden/royal_berkshire_expected.json index a894380..7113251 100644 --- a/tests/golden/royal_berkshire_expected.json +++ b/tests/golden/royal_berkshire_expected.json @@ -39,4 +39,4 @@ "patients_seen_within_weeks": 16, "page_last_updated": "2026-01-26" } -] +] \ No newline at end of file diff --git a/tests/test_extraction.py b/tests/test_extraction.py index 3c25810..a157da9 100644 --- a/tests/test_extraction.py +++ b/tests/test_extraction.py @@ -3,6 +3,8 @@ The keystone test proves the extractor reproduces the golden dataset exactly; the remaining tests pin the edge-case behaviour agreed in the domain model: missing data is a state (None / no record), never an error. +The 2026 layout is covered by its own fixture and golden (added when the +site drifted; the baseline fixture is unchanged per the conftest rule). """ from __future__ import annotations @@ -48,6 +50,39 @@ def test_records_carry_provenance(self, load_fixture): assert record.page_last_updated == date(2026, 1, 26) +class TestLayout2026: + """The 2026 layout: holders + captions + n/a cells + new footer.""" + + def test_fixture_yields_exactly_the_2026_golden(self, load_fixture, load_golden): + page = make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + records = extract_waiting_times(page, region=REGION) + + expected = load_golden("royal_berkshire_2026_expected.json") + assert [record.to_dict() for record in records] == expected + + def test_na_cells_yield_null_waits_not_absent_records(self, load_fixture): + page = make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + records = extract_waiting_times(page, region=REGION) + + first_outpatient = [ + r for r in records if r.metric is Metric.FIRST_OUTPATIENT_APPOINTMENT + ] + assert len(first_outpatient) == 2 + assert all(r.average_wait_weeks is None for r in first_outpatient) + + def test_unavailable_specialty_skipped(self, load_fixture): + page = make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + records = extract_waiting_times(page, region=REGION) + + assert "Paediatric Surgery" not in {r.specialty for r in records} + + def test_footer_date_parsed(self, load_fixture): + page = make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + records = extract_waiting_times(page, region=REGION) + + assert all(r.page_last_updated == date(2026, 8, 7) for r in records) + + class TestEdgeCases: def test_unavailable_specialty_yields_no_records(self, load_fixture): page = make_page( diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 8427638..74a2b5b 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -1,7 +1,8 @@ """Offline tests for the pre-flight layout probe and its pipeline wiring. The drifted fixture proves the probe catches a restructured site; the -pipeline tests prove drift aborts *before* any crawl call is made. +pipeline tests prove drift aborts *before* any crawl call is made. Both +the legacy and the 2026 fixtures must pass the probe. """ from __future__ import annotations @@ -19,6 +20,14 @@ TRUST_URL = "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/" +ALL_FIVE_FAILURES = { + "no

provider heading found", + "no recognised specialty blocks found", + "no recognised metric labels found", + "no waiting-time tables with 'Average waiting time' header found", + "no recognised last-updated footer found", +} + class FakeBackend: def __init__(self, pages_by_seed: dict[str, list[Page]]): @@ -44,17 +53,19 @@ def test_known_good_fixture_passes(self, load_fixture): assert result.ok assert result.failures == () + def test_2026_fixture_passes(self, load_fixture): + result = probe_layout( + make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + ) + + assert result.ok + assert result.failures == () + def test_drifted_fixture_reports_every_failure(self, load_fixture): result = probe_layout(make_page(load_fixture("trust_page_drifted.html"))) assert not result.ok - assert set(result.failures) == { - "no

provider heading found", - "no
blocks found", - "no recognised metric headings (h4) found", - "no waiting-time tables with 'Average waiting time' header found", - "no 'Page last updated: DD/MM/YYYY' footer found", - } + assert set(result.failures) == ALL_FIVE_FAILURES def test_partial_drift_reports_single_failure(self, load_fixture): html = load_fixture("trust_page_royal_berkshire.html").replace( @@ -63,7 +74,7 @@ def test_partial_drift_reports_single_failure(self, load_fixture): result = probe_layout(make_page(html)) assert not result.ok - assert result.failures == ("no 'Page last updated: DD/MM/YYYY' footer found",) + assert result.failures == ("no recognised last-updated footer found",) def test_structurally_valid_but_unextractable_page_flagged(self): # Table present (structural check passes) but not a sibling of the @@ -116,6 +127,15 @@ def test_good_layout_proceeds_to_golden_output(self, load_fixture, load_golden): assert [record.to_dict() for record in result.records] == expected assert backend.crawl_calls == [TRUST_URL] + def test_2026_layout_proceeds_to_2026_golden(self, load_fixture, load_golden): + page = make_page(load_fixture("trust_page_royal_berkshire_2026.html")) + backend = FakeBackend({TRUST_URL: [page]}) + + result = asyncio.run(run_pipeline(backend, [(TRUST_URL, "South East")])) + + expected = load_golden("royal_berkshire_2026_expected.json") + assert [record.to_dict() for record in result.records] == expected + def test_preflight_disabled_preserves_lenient_behaviour(self, load_fixture): drifted = make_page(load_fixture("trust_page_drifted.html")) backend = FakeBackend({TRUST_URL: [drifted]})