fix: harden South Kesteven scraper and Home Assistant runtime - #2180
Open
Dozi3 wants to merge 14 commits into
Open
fix: harden South Kesteven scraper and Home Assistant runtime#2180Dozi3 wants to merge 14 commits into
Dozi3 wants to merge 14 commits into
Conversation
added 12 commits
July 19, 2026 21:56
Contributor
|
Important Review skippedToo many files! This PR contains 198 files, which is 98 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (198)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2180 +/- ##
==========================================
+ Coverage 86.89% 87.74% +0.85%
==========================================
Files 9 11 +2
Lines 1160 1102 -58
==========================================
- Hits 1008 967 -41
+ Misses 152 135 -17 ☔ View full report in Codecov by Harness. |
Contributor
Author
|
@robbrad This is now ready for maintainer review. All 27 upstream checks pass, including the Python 3.12/3.13/3.14 suites, deterministic integration tests, Codecov, and the full Home Assistant compatibility matrix. The PR description includes the live SKDC validation evidence, cross-council scope, suggested review order, and the release-boundary caveat. I am happy to split the import isolation, SKDC, Home Assistant, and release-workflow changes if that would make review easier. |
6 tasks
Owner
|
Flagging this for dedicated maintainer review rather than folding it into the routine "July Part 2" consolidation (#2189). This PR is a 199-file, +13,506/-3,654 line change touching 83 council adapters, core module resolution, the Home Assistant config-entry migration path, and the release/CI workflow itself. That's a much larger blast radius than a typical council fix or hardening patch, and it's not something that should go through a batch consolidation alongside routine dependency bumps and single-council fixes — it deserves its own focused review pass by @robbrad, ideally reading the commits in the order the PR description itself suggests. Not making any judgment on correctness here — all 26 CI checks do pass — just noting the scope warrants a closer look before merge given how much surface area it touches (including release-process changes). No action taken on this PR; leaving it open for you to review. Generated by Claude Code |
Owner
|
Thanks for the detailed writeup — before this can get a proper review, could you help break down why the diff is ~200 files / +13.5k / -3.6k for what's described as an SKDC scraper + HA runtime hardening fix? Looking at the actual file list, this appears to bundle at least four distinct changes:
Given the size and that this touches the release pipeline and 100+ unrelated councils, would you be open to splitting this into separate PRs — the actual SKDC fix + Selenium import deferral as one, the print-redaction sweep as another, the new test harness and CI/release changes as their own? That would make each piece reviewable on its own merits, and it's hard for a maintainer to sign off on the release-workflow and cross-council changes bundled in with a single-council bugfix. Not passing judgment on whether the individual pieces are good — just flagging that "SKDC hardening" undersells what's actually in the diff, and a maintainer needs to understand why each piece is necessary before merging something this broad. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens the South Kesteven District Council (SKDC) scraper and the
Home Assistant/Selenium path that runs it.
/bindaypage, without the standalone HTTP preflight that could receive a403 even when the browser flow remained usable.
imports the wrong top-level
websocketpackage.non-Selenium councils from loading.
contracts while tightening argument propagation, migrations, refresh
coordination, diagnostics, redaction, and WebDriver cleanup.
runtime validation.
This is follow-up hardening to the SKDC work discussed in #1928 and #2000,
originally reviewed in #2121 and incorporated through #2154. It is not a
resubmission of the original scraper change.
Detailed implementation and validation notes are in
docs/south-kesteven-ha-hardening-candidate.md.Root cause
The reported Home Assistant failure happened before the SKDC scraper ran.
Selenium imports
websocket-clientas the top-level modulewebsocket, butHome Assistant's
/configpath contained another top-level/config/websocket/__init__.py. Python loaded that file instead, and itsparent-relative import failed with:
The failure was consequently presented as a repeatedly retried generic
coordinator error. Independently, SKDC's generic
requests.get()preflightcould receive an environment-dependent 403 before the real browser workflow
was attempted.
What changed
South Kesteven
https://www.southkesteven.gov.uk/bindayand follows only thesupported HTTPS self-service checker.
missing and ambiguous matches fail explicitly.
WebDriver unavailability.
the Home Assistant outer wait to 125 seconds.
finally.Core package
package names instead of modifying
sys.path.websocketbelongs to the installedwebsocket-clientdistribution before Selenium is imported.
browser failures, upstream denial, address mismatch, and site drift.
consistently.
yarldependency.Home Assistant
Boolean options are emitted as flags rather than
--flag=True.legacy PAON/house-number, WebDriver, and Boolean aliases.
guessing address or browser values.
reconfiguration flows.
configuration, and unavailable Selenium.
removes the calendar platform's duplicate initial scrape.
CI and release safety
rollback checks.
Review scope
This is intentionally broader than a one-file council fix: 199 files change
with approximately 13,506 additions and 3,654 deletions, including 122 council
adapters. Most council changes are mechanical import deferral and log
redaction; 83 adapters move Selenium imports out of module scope. That sweep
is what prevents a browser-stack collision from breaking unrelated imports.
Suggested review order:
183afcbefor the functional core, SKDC, and Home Assistant changes.uk_bin_collection/uk_bin_collection/councils/SouthKestevenDistrictCouncil.py.uk_bin_collection/uk_bin_collection/dependency_validation.py,collect_data.py, and shared WebDriver helpers.custom_components/uk_bin_collectionmigration, coordinator, and Repairbehavior.
The branch is based directly on current upstream
master(
36d920520e4a4c64fb6278557d357b948bf95e88), is 14 commits ahead, and is notbehind upstream.
Validation
For the exact runtime/documentation candidate
0dd7060d71998a254812666a6cc7ae5af7e89d43:FA950C5EC71603B28EF6322EDF6A2BFC9E5807BB7191669011A42C2C79D2C5B3.>=3.12,<3.15, releasecontract, and manifest-pinning checks passed.
9139a7a0) passed the focused changed-codeselection on Python 3.12, 3.13, and 3.14: 145 tests on each runtime.
were repository-level metadata checks because Issues are disabled and the
fork has no repository topics.
Commit
07d5de7890a49b24d71f5fdcbc4ff49d710ae6edadds only twoupstream-CI compatibility corrections discovered by the first PR run: the
observed Python 3.13 mapping for HA 2026.1/2026.2, and removal of calendar-test
autospeccing that evaluated an unresolved
EntityPlatformannotation insideHA 2026.4/2026.5.
Current branch tip
fc133d3cbed66482a659edf24715645bb4f9eb46adds only focusedbehavioral tests for the coverage gaps reported by that first run. In a clean
checkout, the exact non-live suite passed 348 tests (2 deselected) and measured
88% project coverage, above the 86.89% upstream baseline. The release scripts
reached 100%,
collect_data.py99%, shared helpers 95%, and SKDC 89%. NeitherCI follow-up commit changes integration runtime code; the upstream matrix is
rerun by this PR.
At this head, all 27 upstream checks pass, including Python 3.12/3.13/3.14
unit tests, deterministic integration tests, CodeQL, HACS, HassFest, release
prerequisites, Codecov patch/project gates, and Home Assistant 2025.12.5,
2026.1.3 through 2026.7.2, and
dev.The exact runtime candidate was also exercised in a controlled, reversible
trial on Home Assistant 2026.7.2/Python 3.14 with remote Selenium 4.46.0:
and calendar data;
tracebacks did not recur; and
No private address, token, server address, backup location, or raw live log is
included in this PR.
Compatibility and limitations
councils are covered by deterministic tests and import isolation; this is
not a claim that every external council website was live-tested.
synchronization for Welsh, Irish, Scottish Gaelic, and Portuguese.
supplier adapters, and registry caching remain out of scope.
Release boundary
This source branch is not itself a HACS release. Its development manifest
still requests
uk-bin-collection>=0.170.6, while the already-published0.170.6 wheel predates this candidate. The successful live trial therefore
used a separately built, SHA-256-pinned prerelease wheel.
The added release workflow is intended to handle the durable path atomically:
assign a new version, publish and verify the matching core artifact, exact-pin
the component manifest, and only then publish the component/tag. The branch
should not be manually tagged or advertised as directly installable before
that release sequence completes.