Display place websites and reservation links#117
Conversation
|
Warning Review limit reached
More reviews will be available in 32 minutes and 34 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis PR adds first-class support for place websites and reservation links across the full data pipeline: from Google Maps scraping and API integration, through data normalization and enrichment, to frontend UI rendering. New data models, scraper logic, backend normalization, and component updates work together to expose website URLs and reservation provider links to users. ChangesWebsite and Reservation Links Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
This PR extends the place data pipeline end-to-end to extract, normalize, persist, and display place websites and reservation-provider links (including updates to the vendored Google Maps scraper and the UI place cards).
Changes:
- Vendored scraper: extract
website+reservation_linksfrom the place panel and reservation dialogs, and normalize/dedupe them. - Data pipeline + models: carry
websiteandreservation_linksthrough enrichment, normalized JSON, provenance, and API merge logic. - UI: add “Website” and “Find a table” action buttons on place cards; update CSS and TypeScript types accordingly.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/gmaps-scraper/tests/test_place_scraper.py | Adds unit tests covering reservation dialog scraping, dedupe/merge, and normalization behavior. |
| vendor/gmaps-scraper/src/gmaps_scraper/place_scraper.py | Implements reservation link extraction (panel + dialog), merging, and normalization into PlaceDetails. |
| vendor/gmaps-scraper/src/gmaps_scraper/models.py | Introduces PlaceReservationLink model and adds it to PlaceDetails serialization. |
| tests/python/test_build_data.py | Adds tests ensuring website/reservation links flow through guide normalization and enrichment normalization. |
| scripts/pipeline_models.py | Adds pipeline model types for PlaceReservationLink and wires them into enrichment/provenance/normalized models. |
| scripts/build_data.py | Carries website/reservation links through normalization, canonicalization, provenance, and API merge/backfill. |
| src/lib/types.ts | Updates frontend types for Place.website, Place.reservation_links, and provenance fields. |
| src/components/PlaceCard.astro | Adds website + reservation action links with URL sanitization and search text inclusion. |
| src/styles/global.css | Refactors place-card action link styling to support new buttons and hover/focus states. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a5e924ada
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| normalized_url = _normalize_preview_website(url) if "google.com" in parsed.netloc else url | ||
| if normalized_url is None: | ||
| normalized_url = url if parsed.netloc.endswith("google.com") else None |
There was a problem hiding this comment.
When Maps emits booking URLs on a regional Google host such as www.google.co.jp/maps/reserve/... or a regional /url?q=... redirect, this branch treats the URL as a normal provider link because it only recognizes the literal google.com substring. _reservation_link_is_google_reserve below also only filters google.com, so if the dialog contains both a Google Reserve intermediary and a real provider URL, the generated card can keep and prefer the Google wrapper instead of the direct reservation provider. Please normalize/filter all Google Maps domains consistently before dedupe.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b6cb297b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/build_data.py (2)
6866-6875:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve previously scraped reservation links across partial refreshes.
preserve_existing_enrichment()already keeps page-only fields like reviews and about sections when the refreshed result is weaker.reservation_linksfollow the same failure mode, so a transient dialog/open failure on a later refresh will currently delete working booking links from the cache.Proposed fix
if can_preserve_previous_identity: + if not refreshed_place.reservation_links and previous_place.reservation_links: + refreshed_place.reservation_links = [ + link.model_copy() + for link in previous_place.reservation_links + ] + append_unique_reason(preserved_fields, "reservation_links") if not refreshed_place.review_topics and previous_place.review_topics: refreshed_place.review_topics = previous_place.review_topics[:] append_unique_reason(preserved_fields, "review_topics")🤖 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 `@scripts/build_data.py` around lines 6866 - 6875, The code currently preserves review_topics, reviews, and about_sections when a refreshed_place is weaker, but misses reservation_links; add the same preservation logic for reservation_links within the can_preserve_previous_identity block: if not refreshed_place.reservation_links and previous_place.reservation_links then set refreshed_place.reservation_links = previous_place.reservation_links[:] and call append_unique_reason(preserved_fields, "reservation_links"); use the same variables and helper (preserved_fields, append_unique_reason) so behavior matches the existing fields.
8825-8831:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTry the raw Maps candidate before the synthesized locality-biased search URL.
In this branch,
localized_maps_urlcan still carry the strongest identity signal from the saved list. Putting the synthesizedsearch_urlfirst meansfetch_place_page_enrichment()can accept a same-named place before the original Maps URL is ever tried, which risks caching the wrong enrichment for addressless entries.Proposed fix
if should_replace_raw_search_candidates_with_locality_bias( place, localized_maps_url=localized_maps_url, search_url=search_url, ): - candidates = [search_url, localized_maps_url, cid_url] + candidates = [localized_maps_url, search_url, cid_url] return dedupe_urls([*priority_candidates, *[url for url in candidates if url is not None]])🤖 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 `@scripts/build_data.py` around lines 8825 - 8831, The current branch builds candidates as [search_url, localized_maps_url, cid_url] which tries the synthesized search_url before the original localized_maps_url; change the candidates ordering so the raw Maps URL (localized_maps_url) is tried before the synthesized search_url to avoid accepting a same-named place from search before the original Maps URL is fetched — i.e., inside the should_replace_raw_search_candidates_with_locality_bias branch, build candidates with localized_maps_url before search_url (keeping cid_url and priority_candidates same) and return dedupe_urls([*priority_candidates, *[url for url in candidates if url is not None]]) so fetch_place_page_enrichment will attempt localized_maps_url first.
🧹 Nitpick comments (1)
src/components/PlaceCard.astro (1)
43-53: ⚡ Quick winConsider consolidating duplicate URL validation logic.
The new
safeHttpUrlhelper (lines 43-53) and the existingisSafeExternalUrltype guard (lines 117-127) contain nearly identical validation logic for checking http/https protocols. While they serve slightly different purposes (one returns the URL string, the other is a type guard), the core validation is duplicated.♻️ Refactor to share validation logic
+function isValidHttpUrl(value: string | null | undefined): boolean { + if (!value) { + return false; + } + try { + const parsed = new URL(value); + return parsed.protocol === "https:" || parsed.protocol === "http:"; + } catch { + return false; + } +} + function safeHttpUrl(value: string | null | undefined): string | null { - if (!value) { - return null; - } - try { - const url = new URL(value); - return url.protocol === "https:" || url.protocol === "http:" ? url.toString() : null; - } catch { - return null; - } + return isValidHttpUrl(value) ? value : null; } const isSafeExternalUrl = (value: string | null | undefined): value is string => { - if (!value) { - return false; - } - try { - const parsed = new URL(value); - return parsed.protocol === "https:" || parsed.protocol === "http:"; - } catch { - return false; - } + return isValidHttpUrl(value); };Also applies to: 117-127
🤖 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 `@src/components/PlaceCard.astro` around lines 43 - 53, There is duplicated HTTP/HTTPS URL validation between safeHttpUrl and the type guard isSafeExternalUrl; extract the shared protocol-checking logic into a single helper (e.g., validateHttpUrl or hasHttpProtocol) that attempts new URL(value) and returns either a boolean or the normalized URL string, then reimplement safeHttpUrl to call that helper to return the string/null and reimplement isSafeExternalUrl to call the same helper (or a thin wrapper) to perform the type check; update references to safeHttpUrl and isSafeExternalUrl accordingly so all URL validation uses the shared function.
🤖 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 `@scripts/build_data.py`:
- Around line 3328-3332: reservation_links are being wrapped with
google_places_field regardless of origin, causing page-sourced links to be
marked as API-derived after merge; update the block that assigns
provenance.reservation_links (which currently uses normalized.reservation_links
and google_places_field) to first detect the true origin of those links (e.g.,
check normalized.source or whatever flag set by merge_page_place_into_api_entry
that indicates page-sourced vs google_places_api) and: if the links are
page-sourced assign them directly (serialize with model_dump but do not call
google_places_field) so they remain page-sourced in provenance, otherwise keep
the existing google_places_field wrapping for API-derived links; ensure you
reference normalized.reservation_links and provenance.reservation_links in your
change.
In `@vendor/gmaps-scraper/src/gmaps_scraper/place_scraper.py`:
- Around line 1378-1393: The dialog-close detection in the dialogs loop
(variables dialogs, button, cleanLine and the button.click() call that returns
links) is too narrow and can miss reservation/modals; update the matcher to
perform case-insensitive partial/substring matches and include additional
keywords and variants (e.g., close, dismiss, cancel, reservation, reserve, 閉じ,
關閉, 关闭, 닫, ×, × close, modal-close, dismiss) across
aria-label/title/innerText/textContent instead of exact full-string equality,
and also attempt class/name-based heuristics (e.g., classes like close,
modal-close, dismiss) before returning links; after clicking the close
candidate, verify the dialog is gone (poll/check visibility or
isConnected/offsetParent) and retry a few times with short delays before
proceeding so downstream review/about extraction is not blocked.
---
Outside diff comments:
In `@scripts/build_data.py`:
- Around line 6866-6875: The code currently preserves review_topics, reviews,
and about_sections when a refreshed_place is weaker, but misses
reservation_links; add the same preservation logic for reservation_links within
the can_preserve_previous_identity block: if not
refreshed_place.reservation_links and previous_place.reservation_links then set
refreshed_place.reservation_links = previous_place.reservation_links[:] and call
append_unique_reason(preserved_fields, "reservation_links"); use the same
variables and helper (preserved_fields, append_unique_reason) so behavior
matches the existing fields.
- Around line 8825-8831: The current branch builds candidates as [search_url,
localized_maps_url, cid_url] which tries the synthesized search_url before the
original localized_maps_url; change the candidates ordering so the raw Maps URL
(localized_maps_url) is tried before the synthesized search_url to avoid
accepting a same-named place from search before the original Maps URL is fetched
— i.e., inside the should_replace_raw_search_candidates_with_locality_bias
branch, build candidates with localized_maps_url before search_url (keeping
cid_url and priority_candidates same) and return
dedupe_urls([*priority_candidates, *[url for url in candidates if url is not
None]]) so fetch_place_page_enrichment will attempt localized_maps_url first.
---
Nitpick comments:
In `@src/components/PlaceCard.astro`:
- Around line 43-53: There is duplicated HTTP/HTTPS URL validation between
safeHttpUrl and the type guard isSafeExternalUrl; extract the shared
protocol-checking logic into a single helper (e.g., validateHttpUrl or
hasHttpProtocol) that attempts new URL(value) and returns either a boolean or
the normalized URL string, then reimplement safeHttpUrl to call that helper to
return the string/null and reimplement isSafeExternalUrl to call the same helper
(or a thin wrapper) to perform the type check; update references to safeHttpUrl
and isSafeExternalUrl accordingly so all URL validation uses the shared
function.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99b1cc16-5daf-47bc-a1bc-4453c450b564
📒 Files selected for processing (12)
scripts/build_data.pyscripts/pipeline_models.pysite.example/data/cache/places.sqlitesrc/components/PlaceCard.astrosrc/lib/types.tssrc/styles/global.csstests/e2e/guide-filters.spec.tstests/frontend/guideMapInteractions.test.mjstests/python/test_build_data.pyvendor/gmaps-scraper/src/gmaps_scraper/models.pyvendor/gmaps-scraper/src/gmaps_scraper/place_scraper.pyvendor/gmaps-scraper/tests/test_place_scraper.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdf02b907e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/components/GuideMap.astro:1746
- Clicking the new reservation
<summary>control inside a place card will currently also trigger the card-level click handler (because the guard only excludesa, button, …, [role='button']). This can cause unexpected map selection/scroll when users open the reservation menu. Includesummary(or addrole="button"on the summary) in the exclusion selector.
card.addEventListener("click", (event) => {
const target = event.target instanceof Element ? event.target : null;
if (target?.closest("a, button, input, select, textarea, [role='button']")) return;
selectPlace(placeId, { expandCollapsed: true });
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42c4c12bce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93a0112895
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7306446c05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
origin/main, including cached trust signalswebsiteandreservation_linksthrough enrichment, normalized place JSON, provenance, search text, and TypeScript typesEnrichment note
Existing enrichment cache entries need a place-page refresh before they will contain newly scraped
reservation_links. This checkout only has the trackedsite.examplefallback, so I did not mutate a private site pack cache. Run with the real site pack, for example:If you want to avoid raw list refreshes during that pass, run the underlying command with
--skip-enrichment-source-refresh.Validation
uv run python3 -m unittest vendor.gmaps-scraper.tests.test_place_scraper.PlaceScraperTests.test_collect_reservation_dialog_snapshot_clicks_and_reads_provider_links vendor.gmaps-scraper.tests.test_place_scraper.PlaceScraperTests.test_merge_reservation_links_dedupes_overview_and_dialog_links vendor.gmaps-scraper.tests.test_place_scraper.PlaceScraperTests.test_build_place_details_preserves_reservation_links vendor.gmaps-scraper.tests.test_place_scraper.PlaceScraperTests.test_normalize_reservation_links_cleans_google_dialog_labelsuv run python3 -m unittest discover -s tests/python -p 'test_*.py'bun run checkbun run buildbun run build:datavendor/gmaps-scraper/scripts/lint.shvendor/gmaps-scraper/scripts/typecheck.shhttp://markstokyo.com/, reservationTableCheckhttp://sushitokyo-ten.com/, reservationsIkyu,AutoReserve,TableCheckSummary by CodeRabbit
Release Notes
New Features
Style
Tests