feat(events-crawler): per-source adapters + BiblioCommons (Vancouver PL) - #84
Merged
Conversation
Splits the 735-line single-file crawler into a source registry, shared helpers and
per-source adapters, then adds the second adapter. The Tribe extraction is verbatim —
same fetch shape, same mapping, same image tiers — so the five live orgs are unaffected;
verified by dry-running each and comparing row counts (mosaic 25, burnaby-nh 24,
success 14, centre-canada 0 as documented, pirs 2).
Layout:
lib/{constants,types,text,dates,images,genre,relevance}.ts
adapters/{tribe,bibliocommons}.ts
index.ts — registry + handler only
dryrun.ts — read-only preview harness
BiblioCommons supports no date filtering and no sorting: startDate, endDate, from/to,
start/end, minDate, after, dateRange, sort and sortBy are all silently ignored, the
returned order is arbitrary, and `limit` caps at 100. "The soonest 25 events in the
window" therefore cannot be expressed as a query, so the adapter pages the whole catalog
(VPL: 1,997 events over 20 pages, 4 concurrent), filters and sorts client-side, then caps.
Images resolve only for the final 25 — probing thousands would dominate the run. A
MAX_PAGES backstop bounds a growing catalog and logs when it truncates, so a partial
crawl never looks complete.
Burnaby Public Library is NOT included, and the dry-run harness is why. `bpl` on
BiblioCommons is BOSTON Public Library: it returns a healthy feed of Copley Square,
Allston Brighton CDC and Rian Immigrant Center events that would have looked entirely
plausible in a log while putting Massachusetts events into a BC newcomer app. Burnaby's
real tenant is `burnaby`, which answers "The Events feature is not available", and
bpl.bc.ca/events is a client-rendered SPA with nothing server-side to read. The adapter
now carries a caution about tenant slugs, and index.ts records why Burnaby is absent.
Adds the settlement-relevance filter for library sources (lib/relevance.ts): grouped
keyword terms matched against the accent-folded title only. Accent folding is
load-bearing — Surrey publishes "WorkBC Résumé Clinic" and NVDPL "Tech Café", neither of
which matches a plain ASCII term. Descriptions are deliberately not matched: library
blurbs routinely end "newcomer families welcome", which pulls in every storytime. The
five settlement agencies are never filtered. Measured keep-rates: West Van 8/50,
VPL 112/1997 in-window-and-relevant.
Drops `// @ts-nocheck` from the crawler, so CI's `deno check` genuinely type-checks it.
Confirmed the gate now bites: an injected `const x: number = "str"` fails with TS2322
(exit 1), where previously it exited 0.
Everything new 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 → clean
npx eslint next.config.ts → clean
dryrun all 7 sources → counts above; vpl 25 of 112 relevant
git diff --name-only origin/main | grep -E 'migrations|backfills' → empty
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughThe events crawler now supports typed source adapters for Tribe and BiblioCommons. Shared modules provide date, text, genre, relevance, and image processing. The crawler shares run context across sources, and a dry-run harness previews adapter output without database writes. ChangesEvents crawler expansion
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/functions/events-crawler/lib/relevance.ts (1)
22-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the new shared crawler utilities. These five modules are pure functions with subtle, load-bearing edge-case logic (regex precedence, date-rollover math, entity decoding, deterministic hashing), and none of them ship with tests in this PR. A single silent regression in rule order or an off-by-one in date math would be hard to catch without dedicated coverage.
supabase/functions/events-crawler/lib/relevance.ts#L22-L76: add tests forisSettlementRelevant, covering accent-folded terms (e.g. "Résumé") and the documented exclusions (bareorientation,health,family,community).supabase/functions/events-crawler/lib/genre.ts#L46-L67: add tests forgenreForEventasserting the documented rule-order dependencies (Employment before Language/Family, Housing's word-boundary against "Belkin House").supabase/functions/events-crawler/lib/dates.ts#L37-L62: add tests forwindowEndInTimezone, including the documented month-overflow case (Oct 31 + 4 months → Mar 3).supabase/functions/events-crawler/lib/text.ts#L18-L93: add tests fordecodeEntities's out-of-range numeric entities andisOnlineVenueName's whole-name-match behavior.supabase/functions/events-crawler/lib/images.ts#L35-L67: add tests forhashStr/fallbackImagedeterminism andpexelsQueryForEvent's keyword-ordering.🤖 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/lib/relevance.ts` around lines 22 - 76, Add unit coverage for the shared crawler utilities: in supabase/functions/events-crawler/lib/relevance.ts:22-76, test isSettlementRelevant with accent folding and exclusions for bare orientation, health, family, and community; in supabase/functions/events-crawler/lib/genre.ts:46-67, test genreForEvent rule ordering and Housing’s boundary against “Belkin House”; in supabase/functions/events-crawler/lib/dates.ts:37-62, test windowEndInTimezone including October 31 plus four months rolling to March 3; in supabase/functions/events-crawler/lib/text.ts:18-93, test out-of-range numeric entity decoding and whole-name matching in isOnlineVenueName; and in supabase/functions/events-crawler/lib/images.ts:35-67, test deterministic hashStr/fallbackImage results and keyword ordering in pexelsQueryForEvent. Use the repository’s existing test conventions and assert the documented edge-case behavior.
🤖 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/bibliocommons.ts`:
- Around line 99-119: Update fetchEvents to track pages where fetchPage returns
null during pagination, incrementing the failure count for each failed page.
Include that count in the completion summary or emit a console.warn when
nonzero, while preserving the existing MAX_PAGES truncation reporting.
- Around line 134-136: Update the title handling in the event processing flow to
run the relevance check on the full cleaned value before applying
MAX_TITLE_CHARS truncation. Preserve the empty-title skip, then truncate the
accepted title for storage and keep the existing source.relevanceFilter and
isSettlementRelevant behavior.
- Around line 196-212: Deduplicate the accumulated candidates by event id before
sorting and applying MAX_PER_ORG in the catalog-fetch flow. Update the logic
after candidatesFromPage aggregation to retain only one candidate per id, then
sort and slice the deduplicated collection so duplicates cannot consume slots or
trigger repeated cover resolution.
In `@supabase/functions/events-crawler/adapters/tribe.ts`:
- Around line 162-165: Update the event-row aggregation around tribeEventToRow
to use Promise.allSettled instead of Promise.all, then retain only fulfilled
results whose values are non-null EventRow entries. Preserve the existing
MAX_PER_ORG limit and ensure a rejected per-event conversion is ignored without
causing fetchEvents to return an empty source-wide result.
In `@supabase/functions/events-crawler/dryrun.ts`:
- Around line 32-82: Extract the duplicated crawler wiring into side-effect-free
shared modules: create lib/sources.ts containing the SOURCES registry and
ADAPTERS map, and create a shared context factory for the AdapterContext
construction. In supabase/functions/events-crawler/dryrun.ts lines 32-82, remove
the local definitions and import the shared symbols; at lines 126-133, replace
the inline context literal with the factory. In
supabase/functions/events-crawler/index.ts lines 159-174, remove the duplicated
registry, adapter map, and context construction, importing and using the shared
definitions instead.
---
Outside diff comments:
In `@supabase/functions/events-crawler/lib/relevance.ts`:
- Around line 22-76: Add unit coverage for the shared crawler utilities: in
supabase/functions/events-crawler/lib/relevance.ts:22-76, test
isSettlementRelevant with accent folding and exclusions for bare orientation,
health, family, and community; in
supabase/functions/events-crawler/lib/genre.ts:46-67, test genreForEvent rule
ordering and Housing’s boundary against “Belkin House”; in
supabase/functions/events-crawler/lib/dates.ts:37-62, test windowEndInTimezone
including October 31 plus four months rolling to March 3; in
supabase/functions/events-crawler/lib/text.ts:18-93, test out-of-range numeric
entity decoding and whole-name matching in isOnlineVenueName; and in
supabase/functions/events-crawler/lib/images.ts:35-67, test deterministic
hashStr/fallbackImage results and keyword ordering in pexelsQueryForEvent. Use
the repository’s existing test conventions and assert the documented edge-case
behavior.
🪄 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: c326baed-7cf0-436a-8323-80d623e34545
📒 Files selected for processing (12)
next.config.tssupabase/functions/events-crawler/adapters/bibliocommons.tssupabase/functions/events-crawler/adapters/tribe.tssupabase/functions/events-crawler/dryrun.tssupabase/functions/events-crawler/index.tssupabase/functions/events-crawler/lib/constants.tssupabase/functions/events-crawler/lib/dates.tssupabase/functions/events-crawler/lib/genre.tssupabase/functions/events-crawler/lib/images.tssupabase/functions/events-crawler/lib/relevance.tssupabase/functions/events-crawler/lib/text.tssupabase/functions/events-crawler/lib/types.ts
All five findings applied; each was clearly correct and small. 1. Extract lib/sources.ts (Major, maintainability). index.ts calls Deno.serve at module scope, which is why dryrun.ts had been copying the registry, adapter map and context construction — a drift risk that would quietly stop the preview predicting a real run. The wiring now lives in one side-effect-free module both import, with makeContext() as the single context factory. This was called out as a known trade-off in the PR body; CodeRabbit was right that it is simply fixable. 2. Use Promise.allSettled for per-event mapping (Major, stability). The row mappers await network work (cover HEAD probe, Pexels lookup). Under Promise.all a single rejection rejected the whole batch, hit the outer catch and returned [] — one transient image failure would have discarded every event of that source for the entire weekly run. Applied to both adapters; a failed row is logged and skipped. 3. Run the relevance filter on the full title, then truncate (Minor, correctness). The title was sliced to MAX_TITLE_CHARS before filtering, so a keyword past the cutoff was invisible and the event silently dropped. Both adapters. 4. Dedupe BiblioCommons candidates by event id before capping (Minor, data integrity). The catalog has no stable order and is read over ~20 round trips, so a shift mid-pagination can surface one event on two pages; a duplicate would consume a MAX_PER_ORG slot and pay for a redundant cover probe. 5. Count and report failed page fetches (Trivial). A failed page shrinks the candidate pool invisibly — the summary log looked identical to a complete crawl. Now warns, the same way MAX_PAGES truncation already did. Behaviour is unchanged: all seven sources return exactly the same row counts as before (25/24/14/0/2, westvan 5, vpl 25), and vpl now additionally reports 112 unique of 112. Verification: deno check (incl. dryrun.ts) → clean npx tsc --noEmit / eslint → clean dryrun × 7 sources → counts unchanged migrations/backfills guard → empty Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Applied all five findings in 85be8c8 — each was clearly correct and small, so no follow-up questions.
Behaviour is unchanged — all seven sources return identical row counts (25/24/14/0/2, westvan 5, vpl 25); vpl's summary now also reports @coderabbitai review |
|
The changes address the reported failure handling, filter ordering, BiblioCommons deduplication, page-fetch reporting, and shared source registry. 🐇 ✅ ✅ Action performedReview finished.
|
This was referenced Jul 31, 2026
This was referenced Aug 3, 2026
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
1 participant
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.
What & why
Second of four Phase-2 PRs. Splits the 735-line single-file crawler into a source registry, shared helpers and per-source adapters, then lands the second adapter (BiblioCommons → Vancouver Public Library) on top of it.
The Tribe extraction is verbatim — same fetch shape, same mapping, same three image tiers — so the five live orgs are unaffected. Verified by dry-running every source and comparing row counts:
🛑 The Burnaby catch — why BPL is not in this PR
The plan called for VPL and Burnaby PL. Burnaby is deliberately absent, and the dry-run harness is the reason it was caught before merge rather than after deploy:
bplon BiblioCommons is BOSTON Public Library. It returns a completely healthy feed — "Cover Letter 101", "Citizenship Exam Prep Class", "Legal Clinic for Immigration Assistance with Rian Immigrant Center", at "Central Library in Copley Square". Every one of those is topically perfect for a newcomer app and geographically useless in Metro Vancouver, and nothing in a log would have looked wrong.Burnaby's real tenant is
burnaby, which answers"The Events feature is not available at Burnaby Public Library". Its ownbpl.bc.ca/eventspage is a client-rendered SPA with nothing server-side to read. Burnaby PL cannot be crawled by any approach in this plan and is dropped from scope; the adapter now carries a caution about tenant slugs andindex.tsrecords why it's absent.Decisions baked in
Full pagination, client-side sort. BiblioCommons supports no date filtering and no sorting —
startDate,endDate,from/to,start/end,minDate,after,dateRange,sort,sortByare all silently ignored (countstays at the full catalog size), the order is arbitrary, andlimitcaps at 100. So "the soonest 25 in the window" isn't expressible as a query: the adapter pages the whole catalog (VPL 1,997 events / 20 pages, 4 concurrent), filters and sorts client-side, then caps. Taking only the first few pages would be far cheaper but would yield an arbitrary subset with no way to tell from the result. AMAX_PAGESbackstop bounds catalog growth and logs when it truncates, so a partial crawl never reads as a complete one.Images resolve only for the final 25. The three-tier resolver does a HEAD probe per image; probing every candidate would dominate the run. BiblioCommons
featuredImages taggedEventTypeare shared category placeholders (the sameactivities-and-games.pngacross dozens of unrelated events), so those are skipped in favour of a topic-matched Pexels photo.Relevance filter (
lib/relevance.ts) — grouped keyword terms, matched against the accent-folded title only. Folding is load-bearing: Surrey publishes "WorkBC Résumé Clinic" and NVDPL "Tech Café", neither of which matches a plain ASCII term. Descriptions are deliberately not matched — library blurbs routinely close with "newcomer families welcome", which pulls in every storytime. The five settlement agencies are never filtered.// @ts-nocheckdropped from the crawler, so CI'sdeno checkgenuinely type-checks it. Confirmed the gate now bites: an injectedconst x: number = "str"failsTS2322with exit 1, where the same probe previously exited 0.Changes
supabase/functions/events-crawler/lib/—constants,types,text,dates,images,genre,relevance.supabase/functions/events-crawler/adapters/—tribe.ts(extracted),bibliocommons.ts(new).index.ts— registry + handler only;ADAPTERSis aRecord<SourceKind, Adapter>so a new kind without an implementation is a compile error.dryrun.ts— read-only preview harness. No write path and no--applyflag: it never imports or constructs a Supabase client. It imports the real adapter modules rather than copying their logic, so preview and production cannot drift.next.config.ts— allowlistvpl.bibliocommons.com.Nothing changes in production. New sources stay
enabled: false; no migration, cron job, Vault secret or DB object was touched; the function was not deployed; no writes to shared prod. Activation needs both a deploy and anenabledflip, each gated on Savar's sign-off.Verification
deno check(incl.dryrun.ts)deno checkwith injected type errornpx tsc --noEmitnpx eslint next.config.tsdryrun.ts× 7 sourcesRun the harness yourself:
deno run --allow-net --allow-env=PEXELS_API_KEY supabase/functions/events-crawler/dryrun.ts --source vplReviewer notes
Health, because the sharedGENRE_RULESHealth pattern includesclinic. Correcting it would change classification for the five live orgs, which doesn't belong in this PR. Worth a follow-up; harmless meanwhile since West Van is disabled.dryrun.tsduplicates theSOURCESliteral becauseindex.tscallsDeno.serveat module scope — importing it would start a server. Adding a source means adding it in both places; the harness's--listmakes a drift obvious.lib/dates.tsgainsoffsetIsoToUtcalongsidetoIsoUtc: BiblioCommons'indexStartcarries aZ, and feeding an offset-bearing string to the offset-less parser would silently shift the time.🤖 Generated with Claude Code
Summary by CodeRabbit