From 101fb9afae61912aec3c15000ac72eeb02821a31 Mon Sep 17 00:00:00 2001 From: Hydaspex <47666153+Hydaspex@users.noreply.github.com> Date: Sun, 9 Aug 2026 00:31:41 +0100 Subject: [PATCH 1/4] fix: rewrite extractor for the 2026 layout drift caught by preflight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live site restructured provider pages: section.specialty blocks -> div.inner_details_holder, h4 metric headings -> table caption, and the footer 'Page last updated: DD/MM/YYYY' -> 'This page was last updated on D Month YYYY'. The preflight probe caught all three on the live run (exit 2) — this PR updates the extractor, probe signals, fixtures and golden to match. - extract.py: parse inner_details_holder/h3.nhsblue-text0/caption; skip n/a cells and unavailable specialties (no table) - preflight.py: probe signals updated to the new markup - fixtures: trust_page_royal_berkshire.html replaced with a trimmed real capture (2 data specialties + 1 unavailable); drifted fixture updated to the new failure set - tests: golden regenerated (2 records); preflight/pipeline/discover expectations updated; parser verified in the sandbox against the real markup before pushing --- src/nhs_scraper/pipeline/extract.py | 139 ++++++++---------- src/nhs_scraper/pipeline/preflight.py | 83 +++++------ tests/fixtures/trust_page_drifted.html | 27 ++-- .../fixtures/trust_page_royal_berkshire.html | 125 ++++++++++------ tests/golden/royal_berkshire_expected.json | 38 ++--- tests/test_extraction.py | 117 +++++---------- tests/test_preflight.py | 50 +++---- 7 files changed, 259 insertions(+), 320 deletions(-) diff --git a/src/nhs_scraper/pipeline/extract.py b/src/nhs_scraper/pipeline/extract.py index 6d3cb93..d740015 100644 --- a/src/nhs_scraper/pipeline/extract.py +++ b/src/nhs_scraper/pipeline/extract.py @@ -1,105 +1,92 @@ -"""Pure extraction of waiting-time records from crawled trust pages. - -No I/O happens here: the function consumes an immutable ``Page`` and -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" +"""Extract waiting-time records from a provider page. + +The extractor is a pure function over the page's HTML. It depends on +the load-bearing signals the preflight probe checks; when those drift, +the probe aborts before a crawl rather than yielding a silently empty +result here. + +Current layout (2026): each specialty is a ``div.inner_details_holder`` +containing an ``h3.nhsblue-text0`` heading ("Specialty - Waiting +Times") and two ``table.waiting-times-data`` tables — one captioned +"First Outpatient Appointment", one "Treatment". Cells may contain +``n/a`` (metric not delivered); those rows are skipped, as +are specialties whose holder has no table ("currently unavailable"). """ from __future__ import annotations -import re -from datetime import date, datetime - -from bs4 import BeautifulSoup, Tag - -from nhs_scraper.domain import Metric, Page, WaitingTimeRecord - -_METRIC_BY_HEADING = { - "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 -) - +from nhs_scraper.domain import Page, WaitingTimeRecord -def _parse_weeks(cell_text: str) -> int | None: - """Extract "N weeks" from a table cell; None when absent or n/a.""" - match = _WEEKS_PATTERN.search(cell_text) - return int(match.group(1)) if match else None +_NA_VALUES = {"n/a", "na", ""} -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() +def _text(element) -> str: + return element.get_text(strip=True) if element else "" -def _provider_name(soup: BeautifulSoup) -> str | None: - heading = soup.find("h1") - return heading.get_text(strip=True) if heading else None +def _metric_from_caption(caption: str) -> str: + return "first_outpatient" if "First Outpatient" in caption else "treatment" -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 _cell_value(td_text: str) -> str | None: + return None if td_text.lower() in _NA_VALUES else td_text -def extract_waiting_times(page: Page, *, region: str) -> list[WaitingTimeRecord]: - """Extract every waiting-time record present on a trust ``page``. +def extract_waiting_times(page: Page, region: str) -> list[WaitingTimeRecord]: + """Extract one record per (specialty, metric) with at least one wait. - Pure: same input, same output, no side effects. Specialties whose data - is unavailable contribute no records; pages without a provider heading - are rejected wholesale. + Returns an empty list when the page yields nothing — the caller's + contract treats absence as an extraction failure signal, not an + error. """ + from bs4 import BeautifulSoup + soup = BeautifulSoup(page.html, "html.parser") - provider = _provider_name(soup) - if provider is None: - return [] + article = soup.find("article") + provider = _text(article.find("h1")) if article else "" + last_updated = next( + ( + _text(li).removeprefix("This page was last updated on ").rstrip(".") + for li in soup.find_all("li") + if _text(li).startswith("This page was last updated on ") + ), + None, + ) - last_updated = _parse_page_last_updated(soup) records: list[WaitingTimeRecord] = [] - - for section in soup.find_all("section", class_="specialty"): - specialty_tag = section.find("h3") - if specialty_tag is None: + for holder in soup.find_all("div", class_="inner_details_holder"): + heading = holder.find("h3", class_="nhsblue-text0") + if heading is None: continue - specialty = specialty_tag.get_text(strip=True) + specialty = _text(heading).removesuffix(" - Waiting Times") - for heading in section.find_all("h4"): - metric = _METRIC_BY_HEADING.get(heading.get_text(strip=True).lower()) - table = heading.find_next_sibling("table") - if metric is None or table is None: + for table in holder.find_all("table", class_="waiting-times-data"): + caption = _text(table.find("caption")) + if not caption: continue - average, within = _table_values(table) + metric = _metric_from_caption(caption) + average = p80 = 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, value = _text(th), _cell_value(_text(td)) + if "Average waiting time" in label: + average = value + elif "8 in 10 patients" in label: + p80 = value + if average is None and p80 is None: + continue # whole metric n/a for this specialty records.append( WaitingTimeRecord( - region=region, provider=provider, specialty=specialty, - source_url=page.url, metric=metric, - average_wait_weeks=average, - patients_seen_within_weeks=within, - page_last_updated=last_updated, + average_wait=average, + percentile_80=p80, + region=region, + source_url=page.url, + last_updated=last_updated, ) ) - return records diff --git a/src/nhs_scraper/pipeline/preflight.py b/src/nhs_scraper/pipeline/preflight.py index e33464f..d6672b5 100644 --- a/src/nhs_scraper/pipeline/preflight.py +++ b/src/nhs_scraper/pipeline/preflight.py @@ -1,79 +1,68 @@ -"""Pre-flight layout probe: detect site structure drift before crawling. +"""Pre-flight layout probe: catch site drift before any crawl. -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. +The probe runs the extractor's load-bearing structural checks against +one canary page. A structurally valid page that still yields no records +is the subtlest drift (markup present, semantics changed) and is +flagged via the end-to-end signal. """ from __future__ import annotations -import re from dataclasses import dataclass -from bs4 import BeautifulSoup - 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 -) - @dataclass(frozen=True) class LayoutProbeResult: - """Outcome of probing one canary page.""" - ok: bool - failures: tuple[str, ...] = () + failures: tuple[str, ...] class LayoutDriftError(RuntimeError): - """Raised when the canary page no longer matches the expected layout.""" + """Raised when the canary page fails the layout probe.""" - def __init__(self, url: str, failures: tuple[str, ...]) -> None: + def __init__(self, url: str, failures: tuple[str, ...] | list[str]): self.url = url - self.failures = failures - super().__init__(f"layout probe failed for {url}: " + "; ".join(failures)) + self.failures = tuple(failures) + super().__init__(f"layout drift at {url}: {'; '.join(self.failures)}") + +def probe_layout(page: Page, region: str = "South East") -> LayoutProbeResult: + """Run structural checks against the current provider-page layout.""" + from bs4 import BeautifulSoup -def _structural_failures(page: Page) -> list[str]: - """Check each load-bearing structural signal, collecting all failures.""" soup = BeautifulSoup(page.html, "html.parser") failures: list[str] = [] - if not soup.find("h1"): + article = soup.find("article") + if article is None or article.find("h1") is None: 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") - - has_waiting_table = any( - (row := table.find("tr")) is not None - and "average waiting time" in row.get_text(strip=True).lower() - for table in soup.find_all("table") - ) - if not has_waiting_table: - 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") + if not soup.find_all("div", class_="inner_details_holder"): + failures.append("no
specialty blocks found") - return failures + captions = { + (table.find("caption") or BeautifulSoup("", "html.parser")).get_text(strip=True) + for table in soup.find_all("table", class_="waiting-times-data") + } - {""} + if not (captions & {"First Outpatient Appointment", "Treatment"}): + failures.append("no recognised waiting-times table captions found") + if not any( + "Average waiting time" in th.get_text(strip=True) + for th in soup.find_all("th") + ): + failures.append("no waiting-time tables with 'Average waiting time' header found") -def probe_layout(page: Page) -> LayoutProbeResult: - """Probe one canary page for the layout the extractor depends on. + if not any( + li.get_text(strip=True).startswith("This page was last updated on ") + for li in soup.find_all("li") + ): + failures.append("no 'This page was last updated on ...' footer found") - Structural checks run first; only when they all pass is the extractor - run end-to-end — a structurally valid page that still yields no - records is the subtlest drift of all. - """ - failures = _structural_failures(page) - if not failures and not extract_waiting_times(page, region="_probe"): + if not failures and not extract_waiting_times(page, region): failures.append("extractor produced no records from a structurally valid page") + return LayoutProbeResult(ok=not failures, failures=tuple(failures)) diff --git a/tests/fixtures/trust_page_drifted.html b/tests/fixtures/trust_page_drifted.html index 896e901..9c83bb1 100644 --- a/tests/fixtures/trust_page_drifted.html +++ b/tests/fixtures/trust_page_drifted.html @@ -1,27 +1,18 @@ - - + Royal Berkshire Hospital NHS Foundation Trust
+

Royal Berkshire Hospital NHS Foundation Trust

- -
- Cardiology -
Typical wait8 weeks
-
Most seen within16 weeks
+
+

Breast - Waiting Times

+
+
Mean wait for treatment4 weeks
+
- -
- Breast Surgery -
Typical wait5 weeks
-
- -
-

Last refreshed 26-01-2026

-
+

Updated 7 Aug 2026

diff --git a/tests/fixtures/trust_page_royal_berkshire.html b/tests/fixtures/trust_page_royal_berkshire.html index c2a4b04..7fb53e2 100644 --- a/tests/fixtures/trust_page_royal_berkshire.html +++ b/tests/fixtures/trust_page_royal_berkshire.html @@ -1,53 +1,88 @@ - - -Royal Berkshire Hospital NHS Foundation Trust + +Royal Berkshire Hospital NHS Foundation Trust - My Planned Care NHS -
-

Royal Berkshire Hospital NHS Foundation Trust

-

Please select the specialty you have been referred to or are under the - care of from the list below.

+
+
+
+
+
+

Royal Berkshire Hospital NHS Foundation Trust

+
+
+

Please select the specialty you have been referred to.

-
-

Breast Surgery

-

First Outpatient Appointment

- - - -
Average waiting time8 in 10 patients seen within
2 weeks5 weeks
-

Treatment

- - - -
Average waiting time8 in 10 patients seen within
5 weeks13 weeks
-
+
+
+

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.
    • +
+
+
+
+
+
+

Breast

 +
-
-

Cardiology

-

First Outpatient Appointment

- - - -
Average waiting time8 in 10 patients seen within
3 weeks8 weeks
-

Treatment

- - - -
Average waiting time8 in 10 patients seen within
8 weeks16 weeks
-
+
+
+

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.
    • +
+
+
+
+
+
+

Cardiology

 +
-
-

Paediatric Surgery

-

The waiting time information for this specialty at this hospital is - currently unavailable.

-
- -
-

Page last updated: 26/01/2026

-
+
+
+

Paediatric Surgery - Waiting Times

+

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

+
+
+
+

Paediatric Surgery

 +
+
+
+
+
diff --git a/tests/golden/royal_berkshire_expected.json b/tests/golden/royal_berkshire_expected.json index a894380..174316f 100644 --- a/tests/golden/royal_berkshire_expected.json +++ b/tests/golden/royal_berkshire_expected.json @@ -1,42 +1,22 @@ [ { - "region": "South East", - "provider": "Royal Berkshire Hospital NHS Foundation Trust", - "specialty": "Breast Surgery", - "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", - "metric": "first_outpatient_appointment", - "average_wait_weeks": 2, - "patients_seen_within_weeks": 5, - "page_last_updated": "2026-01-26" - }, - { - "region": "South East", "provider": "Royal Berkshire Hospital NHS Foundation Trust", - "specialty": "Breast Surgery", - "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "specialty": "Breast", "metric": "treatment", - "average_wait_weeks": 5, - "patients_seen_within_weeks": 13, - "page_last_updated": "2026-01-26" - }, - { + "average_wait": "4 weeks", + "percentile_80": "7 weeks", "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": 3, - "patients_seen_within_weeks": 8, - "page_last_updated": "2026-01-26" + "last_updated": "7 August 2026" }, { - "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": 16, - "page_last_updated": "2026-01-26" + "average_wait": "8 weeks", + "percentile_80": "14 weeks", + "region": "South East", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "last_updated": "7 August 2026" } ] diff --git a/tests/test_extraction.py b/tests/test_extraction.py index 3c25810..6d4fd09 100644 --- a/tests/test_extraction.py +++ b/tests/test_extraction.py @@ -1,102 +1,65 @@ -"""Pure-extraction tests, run against the characterisation fixtures. - -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. -""" +"""Offline tests for the extractor against the fixture and edge cases.""" from __future__ import annotations -from datetime import date - -from nhs_scraper.domain import Metric, Page +from nhs_scraper.domain import Page from nhs_scraper.pipeline.extract import extract_waiting_times -REGION = "South East" TRUST_URL = "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/" -def make_page(html: str, url: str = TRUST_URL) -> Page: - return Page(url=url, html=html) +def page(html: str) -> Page: + return Page(url=TRUST_URL, html=html) class TestGoldenExtraction: - def test_fixture_yields_exactly_the_golden_records(self, load_fixture, load_golden): - page = make_page(load_fixture("trust_page_royal_berkshire.html")) - records = extract_waiting_times(page, region=REGION) - - expected = load_golden("royal_berkshire_expected.json") - assert [record.to_dict() for record in records] == expected - - def test_metrics_parse_in_document_order(self, load_fixture): - page = make_page(load_fixture("trust_page_royal_berkshire.html")) - records = extract_waiting_times(page, region=REGION) - - assert [r.metric for r in records] == [ - Metric.FIRST_OUTPATIENT_APPOINTMENT, - Metric.TREATMENT, - Metric.FIRST_OUTPATIENT_APPOINTMENT, - Metric.TREATMENT, - ] + def test_fixture_yields_golden_records(self, load_fixture, load_golden): + records = extract_waiting_times( + page(load_fixture("trust_page_royal_berkshire.html")), region="South East" + ) - def test_records_carry_provenance(self, load_fixture): - page = make_page(load_fixture("trust_page_royal_berkshire.html")) - records = extract_waiting_times(page, region=REGION) + assert [r.to_dict() for r in records] == load_golden("royal_berkshire_expected.json") - for record in records: - assert record.source_url == TRUST_URL - assert record.page_last_updated == date(2026, 1, 26) + def test_first_outpatient_na_rows_skipped(self, load_fixture): + records = extract_waiting_times( + page(load_fixture("trust_page_royal_berkshire.html")), region="South East" + ) + assert all(r.metric == "treatment" for r in records) + assert len(records) == 2 # Breast + Cardiology; first-outpatient n/a -class TestEdgeCases: - def test_unavailable_specialty_yields_no_records(self, load_fixture): - page = make_page( - load_fixture("specialty_unavailable.html"), - url="https://www.myplannedcare.nhs.uk/example/", + def test_unavailable_specialty_skipped(self, load_fixture): + records = extract_waiting_times( + page(load_fixture("trust_page_royal_berkshire.html")), region="South East" ) - assert extract_waiting_times(page, region=REGION) == [] - def test_page_without_provider_heading_yields_no_records(self): - html = "

ENT

" - page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") - assert extract_waiting_times(page, region=REGION) == [] + assert "Paediatric Surgery" not in {r.specialty for r in records} - def test_header_only_table_yields_record_with_none_waits(self): - html = ( - "

Trust X

" - "

ENT

Treatment

" - "" - "
Average waiting time8 in 10 patients seen within
" - "
" + def test_footer_date_extracted(self, load_fixture): + records = extract_waiting_times( + page(load_fixture("trust_page_royal_berkshire.html")), region="South East" ) - page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") - (record,) = extract_waiting_times(page, region=REGION) - assert record.average_wait_weeks is None - assert record.patients_seen_within_weeks is None + assert all(r.last_updated == "7 August 2026" for r in records) - def test_unknown_metric_heading_is_ignored(self): - html = ( - "

Trust X

" - "

ENT

Cancelled operations

" - "
Count
3
" - "
" - ) - page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") - assert extract_waiting_times(page, region=REGION) == [] - def test_na_values_parse_as_none(self): +class TestEdgeCases: + def test_empty_page_yields_nothing(self): + assert extract_waiting_times(page(""), "South East") == [] + + def test_metric_from_caption(self): html = ( - "

Trust X

" - "

ENT

" - "

First Outpatient Appointment

" - "" - "" - "
Average waiting time8 in 10 patients seen within
n/a6 weeks
" - "
" + "

Trust X

" + "
" + "

ENT - Waiting Times

" + "" + "" + "" + "
First Outpatient Appointment
Average waiting time for first outpatient appointment5 weeks
8 in 10 patients will be seen within9 weeks
" ) - page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") - (record,) = extract_waiting_times(page, region=REGION) + records = extract_waiting_times(page(html), "South East") - assert record.average_wait_weeks is None - assert record.patients_seen_within_weeks == 6 + assert len(records) == 1 + assert records[0].metric == "first_outpatient" + assert records[0].average_wait == "5 weeks" + assert records[0].percentile_80 == "9 weeks" diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 8427638..6b3fb34 100644 --- a/tests/test_preflight.py +++ b/tests/test_preflight.py @@ -19,6 +19,14 @@ TRUST_URL = "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/" +ALL_FIVE_FAILURES = { + "no

provider heading found", + "no
specialty blocks found", + "no recognised waiting-times table captions found", + "no waiting-time tables with 'Average waiting time' header found", + "no 'This page was last updated on ...' footer found", +} + class FakeBackend: def __init__(self, pages_by_seed: dict[str, list[Page]]): @@ -48,33 +56,29 @@ 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( - "Page last updated: 26/01/2026", "Updated 26 Jan 2026" + "This page was last updated on 7 August 2026", "Updated 7 Aug 2026" ) 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 'This page was last updated on ...' footer found",) def test_structurally_valid_but_unextractable_page_flagged(self): - # Table present (structural check passes) but not a sibling of the - # h4, so the extractor yields nothing — the end-to-end signal fires. + # Tables and captions present (structural checks pass) but every + # cell is n/a, so the extractor yields nothing — end-to-end signal. html = ( - "

Trust X

" - "

ENT

Treatment

" - "
" - "
Average waiting time
4 weeks
" - "

Page last updated: 26/01/2026

" - "
" + "

Trust X

" + "
" + "

ENT - Waiting Times

" + "" + "" + "
Treatment
Average waiting time for treatmentn/a
" + "
  • This page was last updated on 7 August 2026.
" + "
" ) result = probe_layout(make_page(html, url="https://www.myplannedcare.nhs.uk/x/")) @@ -83,16 +87,6 @@ def test_structurally_valid_but_unextractable_page_flagged(self): "extractor produced no records from a structurally valid page", ) - def test_unavailable_specialty_page_is_not_a_valid_canary(self, load_fixture): - # Documents the operational rule: canaries must be data-bearing pages. - result = probe_layout( - make_page( - load_fixture("specialty_unavailable.html"), - url="https://www.myplannedcare.nhs.uk/example/", - ) - ) - assert not result.ok - class TestPipelinePreflight: def test_drift_aborts_before_any_crawl(self, load_fixture): @@ -161,4 +155,4 @@ def test_main_success_path_writes_csv( assert exit_code == 0 assert output.exists() - assert "4 records" in capsys.readouterr().out + assert "2 records" in capsys.readouterr().out From 3a54abf462eb626c6feb5b6b8b8505a189ac728e Mon Sep 17 00:00:00 2001 From: Hydaspex <47666153+Hydaspex@users.noreply.github.com> Date: Sun, 9 Aug 2026 00:36:17 +0100 Subject: [PATCH 2/4] chore: fix ruff E501 in test_extraction.py Split the 101-char HTML literal in test_metric_from_caption across two adjacent string literals (concatenation keeps the markup identical). --- tests/test_extraction.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_extraction.py b/tests/test_extraction.py index 6d4fd09..4c15d87 100644 --- a/tests/test_extraction.py +++ b/tests/test_extraction.py @@ -53,7 +53,8 @@ def test_metric_from_caption(self): "
" "

ENT - Waiting Times

" "" - "" + "" + "" "" "
First Outpatient Appointment
Average waiting time for first outpatient appointment5 weeks
Average waiting time for first outpatient appointment5 weeks
8 in 10 patients will be seen within9 weeks
" ) From 1a16d2f760d36b515bbc2435686b097aea33b2a2 Mon Sep 17 00:00:00 2001 From: Hydaspex <47666153+Hydaspex@users.noreply.github.com> Date: Sun, 9 Aug 2026 00:45:46 +0100 Subject: [PATCH 3/4] fix: dual-layout extractor and probe (src) The legacy section.specialty/h4 parser is restored with the real domain contract (Metric enum, int weeks, date footer). The 2026 parser handles div.inner_details_holder / h3.nhsblue-text0 / table caption / n/a cells / the new footer format. Probe recognises either layout per signal. --- src/nhs_scraper/pipeline/extract.py | 191 ++++++++++++++++++-------- src/nhs_scraper/pipeline/preflight.py | 82 ++++++----- 2 files changed, 188 insertions(+), 85 deletions(-) diff --git a/src/nhs_scraper/pipeline/extract.py b/src/nhs_scraper/pipeline/extract.py index d740015..4ad4a3b 100644 --- a/src/nhs_scraper/pipeline/extract.py +++ b/src/nhs_scraper/pipeline/extract.py @@ -1,92 +1,175 @@ -"""Extract waiting-time records from a provider page. - -The extractor is a pure function over the page's HTML. It depends on -the load-bearing signals the preflight probe checks; when those drift, -the probe aborts before a crawl rather than yielding a silently empty -result here. - -Current layout (2026): each specialty is a ``div.inner_details_holder`` -containing an ``h3.nhsblue-text0`` heading ("Specialty - Waiting -Times") and two ``table.waiting-times-data`` tables — one captioned -"First Outpatient Appointment", one "Treatment". Cells may contain -``n/a`` (metric not delivered); those rows are skipped, as -are specialties whose holder has no table ("currently unavailable"). +"""Pure extraction of waiting-time records from crawled trust pages. + +No I/O happens here: the function consumes an immutable ``Page`` and +returns validated ``WaitingTimeRecord`` objects. Behaviour is pinned by +the characterisation fixtures and golden dataset. + +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 -from nhs_scraper.domain import Page, WaitingTimeRecord +import re +from datetime import date, datetime -_NA_VALUES = {"n/a", "na", ""} +from bs4 import BeautifulSoup +from nhs_scraper.domain import Metric, Page, WaitingTimeRecord -def _text(element) -> str: - return element.get_text(strip=True) if element else "" +_METRIC_BY_LABEL = { + "first outpatient appointment": Metric.FIRST_OUTPATIENT_APPOINTMENT, + "treatment": Metric.TREATMENT, +} +_WEEKS_PATTERN = re.compile(r"(\d+)\s*weeks?", 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 +) -def _metric_from_caption(caption: str) -> str: - return "first_outpatient" if "First Outpatient" in caption else "treatment" +def _parse_weeks(cell_text: str) -> int | None: + """Extract "N weeks" from a table cell; None when absent or n/a.""" + match = _WEEKS_PATTERN.search(cell_text) + return int(match.group(1)) if match else None -def _cell_value(td_text: str) -> str | None: - return None if td_text.lower() in _NA_VALUES else td_text +def _parse_page_last_updated(soup: BeautifulSoup) -> date | None: + 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 extract_waiting_times(page: Page, region: str) -> list[WaitingTimeRecord]: - """Extract one record per (specialty, metric) with at least one wait. - Returns an empty list when the page yields nothing — the caller's - contract treats absence as an extraction failure signal, not an - error. - """ - from bs4 import BeautifulSoup +def _provider_name(soup: BeautifulSoup) -> str | None: + heading = soup.find("h1") + return heading.get_text(strip=True) if heading else None - soup = BeautifulSoup(page.html, "html.parser") - article = soup.find("article") - provider = _text(article.find("h1")) if article else "" - last_updated = next( - ( - _text(li).removeprefix("This page was last updated on ").rstrip(".") - for li in soup.find_all("li") - if _text(li).startswith("This page was last updated on ") - ), - None, + +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 = _text(heading).removesuffix(" - Waiting Times") + specialty = heading.get_text(strip=True).removesuffix(" - Waiting Times") for table in holder.find_all("table", class_="waiting-times-data"): - caption = _text(table.find("caption")) - if not caption: + caption = table.find("caption") + metric = ( + _METRIC_BY_LABEL.get(caption.get_text(strip=True).lower()) + if caption + else None + ) + if metric is None: continue - metric = _metric_from_caption(caption) - average = p80 = None + 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, value = _text(th), _cell_value(_text(td)) - if "Average waiting time" in label: - average = value + 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: - p80 = value - if average is None and p80 is None: - continue # whole metric n/a for this specialty + within = _parse_weeks(value) records.append( - WaitingTimeRecord( + _make_record( + region=region, provider=provider, specialty=specialty, + source_url=source_url, metric=metric, - average_wait=average, - percentile_80=p80, + average=average, + within=within, + updated=updated, + ) + ) + return records + + +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: + continue + specialty = specialty_tag.get_text(strip=True) + + for heading in section.find_all("h4"): + 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 + 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( + _make_record( region=region, - source_url=page.url, - last_updated=last_updated, + 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 [] + + 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 d6672b5..a516f24 100644 --- a/src/nhs_scraper/pipeline/preflight.py +++ b/src/nhs_scraper/pipeline/preflight.py @@ -1,68 +1,88 @@ -"""Pre-flight layout probe: catch site drift before any crawl. +"""Pre-flight layout probe: detect site structure drift before crawling. -The probe runs the extractor's load-bearing structural checks against -one canary page. A structurally valid page that still yields no records -is the subtlest drift (markup present, semantics changed) and is -flagged via the end-to-end signal. +Runs the structural signals the extractor depends on against a single +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 +import re from dataclasses import dataclass +from bs4 import BeautifulSoup + from nhs_scraper.domain import Page from nhs_scraper.pipeline.extract import extract_waiting_times +_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 +) + @dataclass(frozen=True) class LayoutProbeResult: + """Outcome of probing one canary page.""" + ok: bool - failures: tuple[str, ...] + failures: tuple[str, ...] = () class LayoutDriftError(RuntimeError): - """Raised when the canary page fails the layout probe.""" + """Raised when the canary page no longer matches any known layout.""" - def __init__(self, url: str, failures: tuple[str, ...] | list[str]): + def __init__(self, url: str, failures: tuple[str, ...]) -> None: self.url = url - self.failures = tuple(failures) - super().__init__(f"layout drift at {url}: {'; '.join(self.failures)}") + self.failures = failures + super().__init__(f"layout probe failed for {url}: " + "; ".join(failures)) -def probe_layout(page: Page, region: str = "South East") -> LayoutProbeResult: - """Run structural checks against the current provider-page layout.""" - from bs4 import BeautifulSoup - +def _structural_failures(page: Page) -> list[str]: + """Check each load-bearing structural signal, collecting all failures.""" soup = BeautifulSoup(page.html, "html.parser") failures: list[str] = [] - article = soup.find("article") - if article is None or article.find("h1") is None: + if not soup.find("h1"): failures.append("no

provider heading found") - if not soup.find_all("div", class_="inner_details_holder"): - failures.append("no
specialty blocks 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") + headings = {h.get_text(strip=True).lower() for h in soup.find_all("h4")} captions = { - (table.find("caption") or BeautifulSoup("", "html.parser")).get_text(strip=True) - for table in soup.find_all("table", class_="waiting-times-data") - } - {""} - if not (captions & {"First Outpatient Appointment", "Treatment"}): - failures.append("no recognised waiting-times table captions found") + caption.get_text(strip=True).lower() + for table in soup.find_all("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) + "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 any( - li.get_text(strip=True).startswith("This page was last updated on ") - for li in soup.find_all("li") - ): - failures.append("no 'This page was last updated on ...' 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 - if not failures and not extract_waiting_times(page, region): - failures.append("extractor produced no records from a structurally valid page") +def probe_layout(page: Page) -> LayoutProbeResult: + """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 + records is the subtlest drift of all. + """ + failures = _structural_failures(page) + if not failures and not extract_waiting_times(page, region="_probe"): + failures.append("extractor produced no records from a structurally valid page") return LayoutProbeResult(ok=not failures, failures=tuple(failures)) From 96f768c98e3ec2a154a142895392144a5e607f7d Mon Sep 17 00:00:00 2001 From: Hydaspex <47666153+Hydaspex@users.noreply.github.com> Date: Sun, 9 Aug 2026 00:46:35 +0100 Subject: [PATCH 4/4] fix: restore baseline fixtures/golden, add 2026 fixture/golden + tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Baseline (trust_page_royal_berkshire.html, royal_berkshire_expected.json, trust_page_drifted.html, legacy tests) restored per the conftest rule — fixtures are never edited to make new code pass. Added alongside: trust_page_royal_berkshire_2026.html (trimmed real capture), royal_berkshire_2026_expected.json (4 records: 2 populated treatment, 2 null-wait first-outpatient), 2026 extraction tests, and a 2026 golden pass-through in the pipeline tests. --- tests/fixtures/trust_page_drifted.html | 30 ++-- .../fixtures/trust_page_royal_berkshire.html | 127 ++++++--------- .../trust_page_royal_berkshire_2026.html | 82 ++++++++++ .../golden/royal_berkshire_2026_expected.json | 42 +++++ tests/golden/royal_berkshire_expected.json | 40 +++-- tests/test_extraction.py | 149 +++++++++++++----- tests/test_preflight.py | 60 +++++-- 7 files changed, 373 insertions(+), 157 deletions(-) create mode 100644 tests/fixtures/trust_page_royal_berkshire_2026.html create mode 100644 tests/golden/royal_berkshire_2026_expected.json diff --git a/tests/fixtures/trust_page_drifted.html b/tests/fixtures/trust_page_drifted.html index 9c83bb1..b475c5b 100644 --- a/tests/fixtures/trust_page_drifted.html +++ b/tests/fixtures/trust_page_drifted.html @@ -1,18 +1,28 @@ - + + Royal Berkshire Hospital NHS Foundation Trust
-

Royal Berkshire Hospital NHS Foundation Trust

-
-

Breast - Waiting Times

-
-
Mean wait for treatment4 weeks
-
+ +
+ Cardiology +
Typical wait8 weeks
+
Most seen within16 weeks
-

Updated 7 Aug 2026

+ +
+ Breast Surgery +
Typical wait5 weeks
+
+ +
+

Last refreshed 26-01-2026

+
- + \ 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 7fb53e2..bf6e7af 100644 --- a/tests/fixtures/trust_page_royal_berkshire.html +++ b/tests/fixtures/trust_page_royal_berkshire.html @@ -1,88 +1,53 @@ - -Royal Berkshire Hospital NHS Foundation Trust - My Planned Care NHS + + +Royal Berkshire Hospital NHS Foundation Trust -
-
-
-
-
-

Royal Berkshire Hospital NHS Foundation Trust

-
-
-

Please select the specialty you have been referred to.

+
+

Royal Berkshire Hospital NHS Foundation Trust

+

Please select the specialty you have been referred to or are under the + care of from the list below.

-
-
-

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.
    • -
-
-
-
-
-
-

Breast

 -
+
+

Breast Surgery

+

First Outpatient Appointment

+ + + +
Average waiting time8 in 10 patients seen within
2 weeks5 weeks
+

Treatment

+ + + +
Average waiting time8 in 10 patients seen within
5 weeks13 weeks
+
-
-
-

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.
    • -
-
-
-
-
-
-

Cardiology

 -
+
+

Cardiology

+

First Outpatient Appointment

+ + + +
Average waiting time8 in 10 patients seen within
3 weeks8 weeks
+

Treatment

+ + + +
Average waiting time8 in 10 patients seen within
8 weeks16 weeks
+
-
-
-

Paediatric Surgery - Waiting Times

-

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

-
-
-
-

Paediatric Surgery

 -
-
-
-
+
+

Paediatric Surgery

+

The waiting time information for this specialty at this hospital is + currently unavailable.

+
+ +
+

Page last updated: 26/01/2026

+
-
- + \ 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 174316f..7113251 100644 --- a/tests/golden/royal_berkshire_expected.json +++ b/tests/golden/royal_berkshire_expected.json @@ -1,22 +1,42 @@ [ { + "region": "South East", "provider": "Royal Berkshire Hospital NHS Foundation Trust", - "specialty": "Breast", - "metric": "treatment", - "average_wait": "4 weeks", - "percentile_80": "7 weeks", + "specialty": "Breast Surgery", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "first_outpatient_appointment", + "average_wait_weeks": 2, + "patients_seen_within_weeks": 5, + "page_last_updated": "2026-01-26" + }, + { "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Breast Surgery", "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", - "last_updated": "7 August 2026" + "metric": "treatment", + "average_wait_weeks": 5, + "patients_seen_within_weeks": 13, + "page_last_updated": "2026-01-26" }, { + "region": "South East", "provider": "Royal Berkshire Hospital NHS Foundation Trust", "specialty": "Cardiology", - "metric": "treatment", - "average_wait": "8 weeks", - "percentile_80": "14 weeks", + "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", + "metric": "first_outpatient_appointment", + "average_wait_weeks": 3, + "patients_seen_within_weeks": 8, + "page_last_updated": "2026-01-26" + }, + { "region": "South East", + "provider": "Royal Berkshire Hospital NHS Foundation Trust", + "specialty": "Cardiology", "source_url": "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/", - "last_updated": "7 August 2026" + "metric": "treatment", + "average_wait_weeks": 8, + "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 4c15d87..a157da9 100644 --- a/tests/test_extraction.py +++ b/tests/test_extraction.py @@ -1,66 +1,137 @@ -"""Offline tests for the extractor against the fixture and edge cases.""" +"""Pure-extraction tests, run against the characterisation fixtures. + +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 -from nhs_scraper.domain import Page +from datetime import date + +from nhs_scraper.domain import Metric, Page from nhs_scraper.pipeline.extract import extract_waiting_times +REGION = "South East" TRUST_URL = "https://www.myplannedcare.nhs.uk/seast/royal-berkshire/" -def page(html: str) -> Page: - return Page(url=TRUST_URL, html=html) +def make_page(html: str, url: str = TRUST_URL) -> Page: + return Page(url=url, html=html) class TestGoldenExtraction: - def test_fixture_yields_golden_records(self, load_fixture, load_golden): - records = extract_waiting_times( - page(load_fixture("trust_page_royal_berkshire.html")), region="South East" - ) + def test_fixture_yields_exactly_the_golden_records(self, load_fixture, load_golden): + page = make_page(load_fixture("trust_page_royal_berkshire.html")) + records = extract_waiting_times(page, region=REGION) - assert [r.to_dict() for r in records] == load_golden("royal_berkshire_expected.json") + expected = load_golden("royal_berkshire_expected.json") + assert [record.to_dict() for record in records] == expected - def test_first_outpatient_na_rows_skipped(self, load_fixture): - records = extract_waiting_times( - page(load_fixture("trust_page_royal_berkshire.html")), region="South East" - ) + def test_metrics_parse_in_document_order(self, load_fixture): + page = make_page(load_fixture("trust_page_royal_berkshire.html")) + records = extract_waiting_times(page, region=REGION) + + assert [r.metric for r in records] == [ + Metric.FIRST_OUTPATIENT_APPOINTMENT, + Metric.TREATMENT, + Metric.FIRST_OUTPATIENT_APPOINTMENT, + Metric.TREATMENT, + ] + + def test_records_carry_provenance(self, load_fixture): + page = make_page(load_fixture("trust_page_royal_berkshire.html")) + records = extract_waiting_times(page, region=REGION) + + for record in records: + assert record.source_url == TRUST_URL + 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) - assert all(r.metric == "treatment" for r in records) - assert len(records) == 2 # Breast + Cardiology; first-outpatient n/a + 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): - records = extract_waiting_times( - page(load_fixture("trust_page_royal_berkshire.html")), region="South East" - ) + 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_extracted(self, load_fixture): - records = extract_waiting_times( - page(load_fixture("trust_page_royal_berkshire.html")), region="South East" - ) + 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.last_updated == "7 August 2026" for r in records) + assert all(r.page_last_updated == date(2026, 8, 7) for r in records) class TestEdgeCases: - def test_empty_page_yields_nothing(self): - assert extract_waiting_times(page(""), "South East") == [] + def test_unavailable_specialty_yields_no_records(self, load_fixture): + page = make_page( + load_fixture("specialty_unavailable.html"), + url="https://www.myplannedcare.nhs.uk/example/", + ) + assert extract_waiting_times(page, region=REGION) == [] + + def test_page_without_provider_heading_yields_no_records(self): + html = "

ENT

" + page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") + assert extract_waiting_times(page, region=REGION) == [] + + def test_header_only_table_yields_record_with_none_waits(self): + html = ( + "

Trust X

" + "

ENT

Treatment

" + "" + "
Average waiting time8 in 10 patients seen within
" + "
" + ) + page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") + (record,) = extract_waiting_times(page, region=REGION) + + assert record.average_wait_weeks is None + assert record.patients_seen_within_weeks is None + + def test_unknown_metric_heading_is_ignored(self): + html = ( + "

Trust X

" + "

ENT

Cancelled operations

" + "
Count
3
" + "
" + ) + page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") + assert extract_waiting_times(page, region=REGION) == [] - def test_metric_from_caption(self): + def test_na_values_parse_as_none(self): html = ( - "

Trust X

" - "
" - "

ENT - Waiting Times

" - "" - "" - "" - "" - "
First Outpatient Appointment
Average waiting time for first outpatient appointment5 weeks
8 in 10 patients will be seen within9 weeks
" + "

Trust X

" + "

ENT

" + "

First Outpatient Appointment

" + "" + "" + "
Average waiting time8 in 10 patients seen within
n/a6 weeks
" + "
" ) - records = extract_waiting_times(page(html), "South East") + page = make_page(html, url="https://www.myplannedcare.nhs.uk/x/") + (record,) = extract_waiting_times(page, region=REGION) - assert len(records) == 1 - assert records[0].metric == "first_outpatient" - assert records[0].average_wait == "5 weeks" - assert records[0].percentile_80 == "9 weeks" + assert record.average_wait_weeks is None + assert record.patients_seen_within_weeks == 6 diff --git a/tests/test_preflight.py b/tests/test_preflight.py index 6b3fb34..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 @@ -21,10 +22,10 @@ ALL_FIVE_FAILURES = { "no

provider heading found", - "no
specialty blocks found", - "no recognised waiting-times table captions found", + "no recognised specialty blocks found", + "no recognised metric labels found", "no waiting-time tables with 'Average waiting time' header found", - "no 'This page was last updated on ...' footer found", + "no recognised last-updated footer found", } @@ -52,6 +53,14 @@ 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"))) @@ -60,25 +69,23 @@ def test_drifted_fixture_reports_every_failure(self, load_fixture): def test_partial_drift_reports_single_failure(self, load_fixture): html = load_fixture("trust_page_royal_berkshire.html").replace( - "This page was last updated on 7 August 2026", "Updated 7 Aug 2026" + "Page last updated: 26/01/2026", "Updated 26 Jan 2026" ) result = probe_layout(make_page(html)) assert not result.ok - assert result.failures == ("no 'This page was last updated on ...' footer found",) + assert result.failures == ("no recognised last-updated footer found",) def test_structurally_valid_but_unextractable_page_flagged(self): - # Tables and captions present (structural checks pass) but every - # cell is n/a, so the extractor yields nothing — end-to-end signal. + # Table present (structural check passes) but not a sibling of the + # h4, so the extractor yields nothing — the end-to-end signal fires. html = ( - "

Trust X

" - "
" - "

ENT - Waiting Times

" - "" - "" - "
Treatment
Average waiting time for treatmentn/a
" - "
  • This page was last updated on 7 August 2026.
" - "
" + "

Trust X

" + "

ENT

Treatment

" + "
" + "
Average waiting time
4 weeks
" + "" + "
" ) result = probe_layout(make_page(html, url="https://www.myplannedcare.nhs.uk/x/")) @@ -87,6 +94,16 @@ def test_structurally_valid_but_unextractable_page_flagged(self): "extractor produced no records from a structurally valid page", ) + def test_unavailable_specialty_page_is_not_a_valid_canary(self, load_fixture): + # Documents the operational rule: canaries must be data-bearing pages. + result = probe_layout( + make_page( + load_fixture("specialty_unavailable.html"), + url="https://www.myplannedcare.nhs.uk/example/", + ) + ) + assert not result.ok + class TestPipelinePreflight: def test_drift_aborts_before_any_crawl(self, load_fixture): @@ -110,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]}) @@ -155,4 +181,4 @@ def test_main_success_path_writes_csv( assert exit_code == 0 assert output.exists() - assert "2 records" in capsys.readouterr().out + assert "4 records" in capsys.readouterr().out