feat(events-crawler): Surrey Libraries HTML adapter - #86
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a Surrey Libraries Drupal adapter that fetches and parses events, applies filtering and pagination limits, resolves event metadata, and registers the disabled source with the crawler. ChangesSurrey event ingestion
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
26283a1 to
ccb92b6
Compare
Fourth and final Phase-2 PR. The only HTML-scraping adapter: Surrey publishes no RSS, no
ICS and no JSON, and its BiblioCommons tenant has the Events feature disabled — but
/events?page=N is fully server-rendered, so no headless browser is needed.
It earns the scraping cost. Surrey is by far the most settlement-relevant Phase-2 source:
all 25 rows a run would produce are on-mission — Practice Speaking English (incl.
Advanced), Settlement Services for Newcomers, Settlement & Integration Services,
Employment Services for Newcomers, Employment Services for International Students,
Résumé Clinic with S.U.C.C.E.S.S., Canadian Job Search Workshop, English Conversation
Program (55+). That last set also confirms the relevance filter's accent folding against
live data: "Résumé Clinic" only matches because the title is NFD-folded before testing.
Datetimes come from the <time datetime="…"> attribute rather than the eventdate query
param the plan specified. Both encode the same instant, but the attribute is a full
ISO-8601 timestamp carrying its own offset ("2026-07-31T10:00:00-07:00"), so it needs no
timezone reconstruction and is DST-correct by construction, where eventdate is a bare
local "YYYY-MM-DD HH:MM:SS". eventdate is still kept in external_link, because it is what
makes each occurrence of a recurring program distinct under events_external_link_key.
Pagination is 0-based and chronological, so the walk starts at today and stops on the
first of: listing exhausted, a page past the window, MAX_PER_ORG candidates collected, or
the MAX_PAGES cap. The cap is set at 20 because 10 listings per page against a ~20%
relevance keep-rate would otherwise need ~125 requests to fill the cap. Hitting it warns
rather than truncating silently, and it costs little: the listing is chronological, so the
events beyond the cap are the furthest out and land on a later weekly run.
The listing ships no image, so covers always fall through to the Pexels topic tier and
then the deterministic Unsplash pool — both already allowlisted, so no next.config.ts
change is needed for this source.
Carries the same two fixes as the other adapters: relevance filtering runs on the full
title before truncation, and row mapping uses Promise.allSettled.
Everything stays enabled: false. No migration, cron, or DB object touched; nothing
deployed; no writes to shared prod.
Verification:
deno check (incl. dryrun.ts) → clean
npx tsc --noEmit / eslint → clean
dryrun surrey-libraries → 20 pages, 28 relevant, 25 rows, all on-mission
regression, all 10 sources → 25/24/14/0/2, 5, 25, 2, 16, 25
migrations/backfills guard → empty
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
103f74d to
06b31f4
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 48 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/events-crawler/adapters/surrey.ts`:
- Around line 144-159: Update the Surrey page-walking logic around parsePage so
a zero-block result on page 0 logs a warning instead of being treated as a
legitimate exhausted listing. Preserve normal exhaustion handling for later
pages, while ensuring the warning identifies the parse or markup failure and the
walk does not silently return zero results as a valid empty calendar.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df3a689e-cbd0-4350-b3d3-7bc5da39a871
📒 Files selected for processing (3)
supabase/functions/events-crawler/adapters/surrey.tssupabase/functions/events-crawler/lib/sources.tssupabase/functions/events-crawler/lib/types.ts
…t exhausted parsePage derives `blocks` from splitting on BLOCK_MARKER, so `blocks === 0` collapses two very different states: the listing genuinely ran out, or the marker stopped matching — a Drupal theme renaming the wrapper class, a soft-404, or an interstitial served with HTTP 200. The second is the expected failure mode for a scraping adapter. Treating both as "exhausted" made the failure silent: the walk breaks, `stoppedEarly` suppresses the MAX_PAGES warning, and the run returns zero rows behind an info-level line identical to the one a genuinely empty calendar produces. Nothing in the logs distinguishes "Surrey has no events" from "Surrey's markup moved". Page 0 is never legitimately empty for a live listing, so that case now logs an error naming the marker it failed to find. `pagesWalked === 1` identifies page 0 specifically: the counter increments before the null-check, so a failed page-0 fetch (already logged by fetchPage) can't shift the attribution onto page 1. Log-only. `exhausted` still fires and the walk behaves exactly as before, so crawl semantics, the page cap and the enabled: false gate are all untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Applied in Verified against the code first. On Scope: log-only. |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 48 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/events-crawler/adapters/surrey.ts`:
- Around line 80-130: Update parsePage to track chunks that fail TITLE_LINK_RE
extraction and expose that diagnostic in its PageParse result. In fetchEvents,
immediately after the blocks === 0 handling, detect page 0 where every parsed
block missed title extraction and log a clear parse-failure error instead of
silently reporting zero unique events. Preserve normal behavior when at least
one title matches or on later pages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a88d30d9-35f4-467f-8b32-2d0080c74686
📒 Files selected for processing (3)
supabase/functions/events-crawler/adapters/surrey.tssupabase/functions/events-crawler/lib/sources.tssupabase/functions/events-crawler/lib/types.ts
| interface PageParse { | ||
| candidates: Candidate[]; | ||
| /** Blocks seen, before any filtering — 0 means the listing has run out. */ | ||
| blocks: number; | ||
| /** Latest start on the page, to decide whether the window has been walked past. */ | ||
| maxStartMs: number; | ||
| } | ||
|
|
||
| function parsePage(html: string, source: Source, ctx: AdapterContext): PageParse { | ||
| const chunks = html.split(BLOCK_MARKER).slice(1); | ||
| const candidates: Candidate[] = []; | ||
| let maxStartMs = 0; | ||
|
|
||
| for (const chunk of chunks) { | ||
| const titleMatch = chunk.match(TITLE_LINK_RE); | ||
| if (!titleMatch) continue; | ||
| const href = titleMatch[1]; | ||
| // Filter on the FULL cleaned title, then truncate for storage. | ||
| const fullTitle = clean(titleMatch[2]); | ||
| if (!fullTitle) continue; | ||
|
|
||
| TIME_RE.lastIndex = 0; | ||
| const times = [...chunk.matchAll(TIME_RE)].map((m) => m[1]); | ||
| const startIso = offsetIsoToUtc(times[0]); | ||
| if (!startIso) continue; | ||
| const startMs = Date.parse(startIso); | ||
| if (startMs > maxStartMs) maxStartMs = startMs; | ||
|
|
||
| // Track the window before relevance, so the walk can stop on dates even when a page | ||
| // happens to contain nothing relevant. | ||
| if (source.relevanceFilter && !isSettlementRelevant(fullTitle)) continue; | ||
| if (startMs < ctx.nowMs || startMs > ctx.windowEndMs) continue; | ||
|
|
||
| const locationMatch = chunk.match(LOCATION_RE); | ||
| const location = clean(locationMatch?.[1]); | ||
| if (!location) continue; // events.location is NOT NULL | ||
|
|
||
| let endIso = offsetIsoToUtc(times[1]); | ||
| if (endIso && Date.parse(endIso) <= startMs) endIso = null; | ||
|
|
||
| candidates.push({ | ||
| title: fullTitle.slice(0, MAX_TITLE_CHARS), | ||
| link: `https://${source.host}${href}`, | ||
| startIso, | ||
| endIso, | ||
| location, | ||
| }); | ||
| } | ||
|
|
||
| return { candidates, blocks: chunks.length, maxStartMs }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Generalize the parse-failure diagnostic to title-link extraction.
The fix at lines 148-163 correctly distinguishes a genuine empty listing from a broken BLOCK_MARKER on page 0. TITLE_LINK_RE sits directly downstream of that marker and fails the same way: if Drupal renames the anchor or <h3> markup, every chunk on page 0 fails titleMatch (line 94), candidates stays empty, blocks stays non-zero, and the run logs walked N page(s), 0 ... unique with no error — the exact symptom the block-marker fix was written to prevent, just one regex layer down.
Track title-match misses in parsePage and log when all blocks on page 0 fail to extract a title.
🛠️ Proposed fix to track and log title-extraction failures
interface PageParse {
candidates: Candidate[];
/** Blocks seen, before any filtering — 0 means the listing has run out. */
blocks: number;
+ /** Blocks whose title link/heading markup did not match, out of `blocks`. */
+ titleMisses: number;
/** Latest start on the page, to decide whether the window has been walked past. */
maxStartMs: number;
}
function parsePage(html: string, source: Source, ctx: AdapterContext): PageParse {
const chunks = html.split(BLOCK_MARKER).slice(1);
const candidates: Candidate[] = [];
let maxStartMs = 0;
+ let titleMisses = 0;
for (const chunk of chunks) {
const titleMatch = chunk.match(TITLE_LINK_RE);
- if (!titleMatch) continue;
+ if (!titleMatch) {
+ titleMisses++;
+ continue;
+ }
...
}
- return { candidates, blocks: chunks.length, maxStartMs };
+ return { candidates, blocks: chunks.length, titleMisses, maxStartMs };
}Then in fetchEvents, after the blocks === 0 branch:
candidates.push(...parsed.candidates);
+ if (pagesWalked === 1 && parsed.blocks > 0 && parsed.titleMisses === parsed.blocks) {
+ console.error(
+ `events-crawler: ${source.slug} found ${parsed.blocks} block(s) on page 0 but ` +
+ `none had a matching title link — the title markup has probably changed`,
+ );
+ }
if (parsed.maxStartMs > ctx.windowEndMs) pastWindow = true;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface PageParse { | |
| candidates: Candidate[]; | |
| /** Blocks seen, before any filtering — 0 means the listing has run out. */ | |
| blocks: number; | |
| /** Latest start on the page, to decide whether the window has been walked past. */ | |
| maxStartMs: number; | |
| } | |
| function parsePage(html: string, source: Source, ctx: AdapterContext): PageParse { | |
| const chunks = html.split(BLOCK_MARKER).slice(1); | |
| const candidates: Candidate[] = []; | |
| let maxStartMs = 0; | |
| for (const chunk of chunks) { | |
| const titleMatch = chunk.match(TITLE_LINK_RE); | |
| if (!titleMatch) continue; | |
| const href = titleMatch[1]; | |
| // Filter on the FULL cleaned title, then truncate for storage. | |
| const fullTitle = clean(titleMatch[2]); | |
| if (!fullTitle) continue; | |
| TIME_RE.lastIndex = 0; | |
| const times = [...chunk.matchAll(TIME_RE)].map((m) => m[1]); | |
| const startIso = offsetIsoToUtc(times[0]); | |
| if (!startIso) continue; | |
| const startMs = Date.parse(startIso); | |
| if (startMs > maxStartMs) maxStartMs = startMs; | |
| // Track the window before relevance, so the walk can stop on dates even when a page | |
| // happens to contain nothing relevant. | |
| if (source.relevanceFilter && !isSettlementRelevant(fullTitle)) continue; | |
| if (startMs < ctx.nowMs || startMs > ctx.windowEndMs) continue; | |
| const locationMatch = chunk.match(LOCATION_RE); | |
| const location = clean(locationMatch?.[1]); | |
| if (!location) continue; // events.location is NOT NULL | |
| let endIso = offsetIsoToUtc(times[1]); | |
| if (endIso && Date.parse(endIso) <= startMs) endIso = null; | |
| candidates.push({ | |
| title: fullTitle.slice(0, MAX_TITLE_CHARS), | |
| link: `https://${source.host}${href}`, | |
| startIso, | |
| endIso, | |
| location, | |
| }); | |
| } | |
| return { candidates, blocks: chunks.length, maxStartMs }; | |
| } | |
| interface PageParse { | |
| candidates: Candidate[]; | |
| /** Blocks seen, before any filtering — 0 means the listing has run out. */ | |
| blocks: number; | |
| /** Blocks whose title link/heading markup did not match, out of `blocks`. */ | |
| titleMisses: number; | |
| /** Latest start on the page, to decide whether the window has been walked past. */ | |
| maxStartMs: number; | |
| } | |
| function parsePage(html: string, source: Source, ctx: AdapterContext): PageParse { | |
| const chunks = html.split(BLOCK_MARKER).slice(1); | |
| const candidates: Candidate[] = []; | |
| let maxStartMs = 0; | |
| let titleMisses = 0; | |
| for (const chunk of chunks) { | |
| const titleMatch = chunk.match(TITLE_LINK_RE); | |
| if (!titleMatch) { | |
| titleMisses++; | |
| continue; | |
| } | |
| const href = titleMatch[1]; | |
| // Filter on the FULL cleaned title, then truncate for storage. | |
| const fullTitle = clean(titleMatch[2]); | |
| if (!fullTitle) continue; | |
| TIME_RE.lastIndex = 0; | |
| const times = [...chunk.matchAll(TIME_RE)].map((m) => m[1]); | |
| const startIso = offsetIsoToUtc(times[0]); | |
| if (!startIso) continue; | |
| const startMs = Date.parse(startIso); | |
| if (startMs > maxStartMs) maxStartMs = startMs; | |
| // Track the window before relevance, so the walk can stop on dates even when a page | |
| // happens to contain nothing relevant. | |
| if (source.relevanceFilter && !isSettlementRelevant(fullTitle)) continue; | |
| if (startMs < ctx.nowMs || startMs > ctx.windowEndMs) continue; | |
| const locationMatch = chunk.match(LOCATION_RE); | |
| const location = clean(locationMatch?.[1]); | |
| if (!location) continue; // events.location is NOT NULL | |
| let endIso = offsetIsoToUtc(times[1]); | |
| if (endIso && Date.parse(endIso) <= startMs) endIso = null; | |
| candidates.push({ | |
| title: fullTitle.slice(0, MAX_TITLE_CHARS), | |
| link: `https://${source.host}${href}`, | |
| startIso, | |
| endIso, | |
| location, | |
| }); | |
| } | |
| return { candidates, blocks: chunks.length, titleMisses, maxStartMs }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/events-crawler/adapters/surrey.ts` around lines 80 - 130,
Update parsePage to track chunks that fail TITLE_LINK_RE extraction and expose
that diagnostic in its PageParse result. In fetchEvents, immediately after the
blocks === 0 handling, detect page 0 where every parsed block missed title
extraction and log a clear parse-failure error instead of silently reporting
zero unique events. Preserve normal behavior when at least one title matches or
on later pages.
|
Taking the The finding is valid — if Drupal renames the anchor or One correction to the description, though: it states the run logs So the failure is misattributed (it reads as a page-cap effect rather than a parse break), not silent. That is a real diagnostic gap, and it is materially smaller than the Why defer: the source ships Tracked in |
docs(backlog): track the Surrey parse-failure diagnostic deferred from PR #86
1 participant
What & why
Fourth and final Phase-2 PR. The only HTML-scraping adapter: Surrey publishes no RSS, no ICS and no JSON, and its BiblioCommons tenant has the Events feature disabled — but
/events?page=Nis fully server-rendered, so no headless browser is needed.It earns the scraping cost. Surrey is by far the most settlement-relevant Phase-2 source — all 25 rows a run would produce are on-mission:
That also confirms the relevance filter's accent folding against live data:
Résumé Cliniconly matches because the title is NFD-folded before testing — the exact case you asked to be handled.Decisions baked in
Datetimes come from
<time datetime="…">, not theeventdatequery param the plan specified. Both encode the same instant, but the attribute is a full ISO-8601 timestamp carrying its own offset (2026-07-31T10:00:00-07:00), so it needs no timezone reconstruction and is DST-correct by construction;eventdateis a bare localYYYY-MM-DD HH:MM:SS. Each row carries two<time>elements, giving start and end.eventdateis still kept inexternal_link— it's what makes each occurrence of a recurring program distinct underevents_external_link_key.Page walking stops early. Pagination is 0-based and chronological, so the walk starts at today and stops on the first of: listing exhausted, a page past the window,
MAX_PER_ORGcollected, or theMAX_PAGEScap. The cap is 20 because 10 listings/page against a ~20% keep-rate would otherwise need ~125 requests to fill the cap. Hitting it warns rather than truncating silently, and costs little — the listing is chronological, so events beyond the cap are the furthest out and land on a later weekly run.No
next.config.tschange. The listing ships no image, so covers always fall through to the Pexels topic tier and then the Unsplash pool — both already allowlisted.Changes
adapters/surrey.ts— new.lib/types.ts—SourceKindgainssurrey-drupal.lib/sources.ts— registersurrey-libraries(enabled: false,relevanceFilter: true).Nothing changes in production.
enabled: false; no migration, cron job, Vault secret or DB object touched; the function was not deployed; no writes to shared prod.Verification
deno check(incl.dryrun.ts)npx tsc --noEmit/eslintdryrun --source surrey-librariesReviewer notes
</div>s — each chunk then yields its fields by first-match, which is resilient to the nested views markup Drupal emits around the address field.descriptionisnull: the listing carries no summary and the detail page would be a separate fetch per event. The detail page renders fine without one (the About section hides when empty).🤖 Generated with Claude Code
Summary by CodeRabbit