fix: scraper regex matches new "Last Free Electricity sessions:" heading - #22
fix: scraper regex matches new "Last Free Electricity sessions:" heading#22mindbox77 wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 57 minutes and 58 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request refines error handling and regex pattern matching. The main.py file now distinguishes between failed HTML fetches and successful fetches with no extracted sessions using separate log levels. The scraper_website.py file expands regex patterns to recognize additional heading variants with optional "Free Electricity" text and flexible pluralization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Octopus updated the /free-electricity/ page heading from "Last Session:" to "⚡️Last Free Electricity sessions:⚡️", causing the website scraper's regex to miss all sessions and return 0. Broaden the regex to accept the optional "Free Electricity" phrase and plural "Sessions" for both "Next" and "Last" headings. Also clarify the misleading "Failed to fetch HTML content" log message — it previously fired whenever no sessions were extracted, even on a successful fetch. Now split into a real fetch-failure error and an informational log when the page simply has no sessions listed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
21aa34e to
929c955
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
octofree/main.py (1)
725-728: Log separation is a clear improvement; minor edge case on empty-string content.Splitting fetch failure (error) from "page fetched but empty extraction" (info) directly addresses the misleading logs described in the PR.
Small gap:
fetch_page_contentcurrently only returnsNoneorresponse.text, but if upstream ever returned an empty string (e.g., a future change or unusual 200 response), line 348'sif html_content:would treat it as falsy while line 725'selif html_content is Nonewould be False, so the empty-body case would land in the "no sessions extracted" branch rather than being surfaced as a fetch anomaly. If you want to be defensive, consider:Optional hardening
- elif html_content is None: - logging.error("Failed to fetch HTML content.") - else: + elif not html_content: + logging.error("Failed to fetch HTML content.") + else: logging.info("ℹ️ No sessions extracted from HTML (page may have no sessions listed yet)")Ruff's RUF001 warning about the
ℹcharacter on line 728 is a stylistic false positive here (intentional info glyph) and can be ignored or silenced with a# noqa: RUF001.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@octofree/main.py` around lines 725 - 728, The fetch result handling should distinguish None (fetch failure) from an empty string (empty response); update the block that checks html_content returned by fetch_page_content so it treats html_content is None as the error case, html_content == "" (or len(html_content) == 0) as a separate error/warning like logging.error("Fetched empty HTML response") before falling through to the "no sessions extracted" info branch, and leave the existing logging.info message (the one containing the "ℹ️ No sessions extracted..." text) but append a "# noqa: RUF001" comment to that logging line to silence Ruff's false positive about the info glyph.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@octofree/main.py`:
- Around line 725-728: The fetch result handling should distinguish None (fetch
failure) from an empty string (empty response); update the block that checks
html_content returned by fetch_page_content so it treats html_content is None as
the error case, html_content == "" (or len(html_content) == 0) as a separate
error/warning like logging.error("Fetched empty HTML response") before falling
through to the "no sessions extracted" info branch, and leave the existing
logging.info message (the one containing the "ℹ️ No sessions extracted..." text)
but append a "# noqa: RUF001" comment to that logging line to silence Ruff's
false positive about the info glyph.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c782cdc9-1476-4768-96eb-f5a4b0e2e7da
📒 Files selected for processing (2)
octofree/main.pyoctofree/scraper_website.py
Summary
Last Session:to⚡️Last Free Electricity sessions:⚡️. Broaden the regex inextract_sessionsto accept optionalFree Electricityand pluralSessionsfor bothNextandLastvariants.ERROR: Failed to fetch HTML content.was firing on every cycle where no sessions were extracted — even when the fetch itself succeeded (96KB of HTML was returned). Split into a real fetch-failure error and an informational log when the page simply has no sessions listed yet.Context
Reported from a user's production log showing successful HTML fetches (
length: 96318) but 0 sessions extracted every cycle, followed by the misleadingFailed to fetch HTML contenterror. Verified locally against the current live HTML: the fixed regex correctly extracts9-10pm, Friday 24th Octoberand12-3pm, Saturday 25th October.Test plan
extract_sessionsagainst current live HTML fromhttps://octopus.energy/free-electricity/returns both scheduled sessions🤖 Generated with Claude Code
Summary by CodeRabbit