Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/gmaps_scraper/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class Place:
maps_url: str
cid: str | None = None
google_id: str | None = None
cid_aliases: list[str] = field(default_factory=list)
is_favorite: bool = False
added_by: ListOwner | None = None

Expand All @@ -100,6 +101,8 @@ def to_dict(self) -> dict[str, object]:
del result["note"]
if self.cid is not None:
result["cid"] = self.cid
if self.cid_aliases:
result["cid_aliases"] = self.cid_aliases
if self.google_id is not None:
result["google_id"] = self.google_id
if self.added_by is not None:
Expand Down
128 changes: 121 additions & 7 deletions src/gmaps_scraper/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ class _Candidate:
signal_score: int


@dataclass(frozen=True, slots=True)
class _PlaceDedupeKeys:
primary: set[str]
aliases: set[str]


class ParseError(RuntimeError):
"""Raised when a saved list cannot be parsed from the supplied artifacts."""

Expand Down Expand Up @@ -388,7 +394,8 @@ def _parse_list_owner(node: JSONValue | None) -> ListOwner | None:

def _extract_places(node: JSONValue) -> list[Place]:
places: list[Place] = []
seen: set[str] = set()
primary_key_indexes: dict[str, int] = {}
alias_key_indexes: dict[str, int] = {}

for current, ancestors in _walk_json(node):
if not _is_coordinate_tuple(current):
Expand All @@ -405,6 +412,7 @@ def _extract_places(node: JSONValue) -> list[Place]:
metadata_node = _find_place_metadata(ancestors)
address = _extract_address(metadata_node)
cid = _find_cid(metadata_node)
cid_aliases = _find_cid_aliases(metadata_node)
google_id = _find_google_id(metadata_node)
name = _find_place_name(ancestors, address=address, place_record=place_record)
note = _find_place_note(place_record, name=name, address=address)
Expand All @@ -422,23 +430,95 @@ def _extract_places(node: JSONValue) -> list[Place]:
lng=lng,
),
cid=cid,
cid_aliases=cid_aliases,
google_id=google_id,
is_favorite=is_favorite,
added_by=_find_place_added_by(place_record),
)
dedupe_key = (
google_id
or (f"{cid}:{lat:.6f}:{lng:.6f}" if cid is not None else None)
or f"{place.name}:{lat:.6f}:{lng:.6f}"
dedupe_keys = _place_dedupe_keys(place)
duplicate_index = _duplicate_place_index(
dedupe_keys,
primary_key_indexes=primary_key_indexes,
alias_key_indexes=alias_key_indexes,
)
if dedupe_key in seen:
if duplicate_index is not None:
if _place_dedupe_quality(place) > _place_dedupe_quality(places[duplicate_index]):
places[duplicate_index] = place

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale dedupe keys when replacing alias rows

When an alias-only row is replaced by a higher-quality canonical row, the dictionaries still retain the old row's primary key (for example the S2-cell CID) because this branch only records the new keys. If a later distinct venue at the same coordinates has that same S2 alias plus its own real CID, _duplicate_place_index sees the stale primary alias and drops the later venue; I reproduced this by inserting an alias-only Northwind row before the canonical row, then moving Harbor to the same coordinates with the shared cell alias, and the parser returned only Northwind.

Useful? React with 👍 / 👎.

_record_place_dedupe_keys(
duplicate_index,
dedupe_keys,
primary_key_indexes=primary_key_indexes,
alias_key_indexes=alias_key_indexes,
)
continue
seen.add(dedupe_key)
places.append(place)
_record_place_dedupe_keys(
len(places) - 1,
dedupe_keys,
primary_key_indexes=primary_key_indexes,
alias_key_indexes=alias_key_indexes,
)

return places


def _place_dedupe_keys(place: Place) -> _PlaceDedupeKeys:
primary: set[str] = set()
aliases: set[str] = set()
if place.google_id:
primary.add(f"gid:{place.google_id}")
if place.cid is not None:
cid_suffix = f"{place.lat:.6f}:{place.lng:.6f}"
primary.add(f"cid:{place.cid}:{cid_suffix}")
aliases.update(
f"cid:{cid_alias}:{cid_suffix}"
for cid_alias in place.cid_aliases
)
if not primary:
primary.add(f"name:{place.name}:{place.lat:.6f}:{place.lng:.6f}")
return _PlaceDedupeKeys(primary=primary, aliases=aliases)


def _duplicate_place_index(
keys: _PlaceDedupeKeys,
*,
primary_key_indexes: dict[str, int],
alias_key_indexes: dict[str, int],
) -> int | None:
for key in keys.primary:
if key in primary_key_indexes:
return primary_key_indexes[key]
for key in keys.aliases:
if key in primary_key_indexes:
return primary_key_indexes[key]
for key in keys.primary:
if key in alias_key_indexes:
return alias_key_indexes[key]
return None


def _record_place_dedupe_keys(
index: int,
keys: _PlaceDedupeKeys,
*,
primary_key_indexes: dict[str, int],
alias_key_indexes: dict[str, int],
) -> None:
for key in keys.primary:
primary_key_indexes[key] = index
for key in keys.aliases:
alias_key_indexes[key] = index


def _place_dedupe_quality(place: Place) -> tuple[bool, bool, bool, bool]:
return (
place.google_id is not None,
bool(place.cid_aliases),
place.cid is not None,
place.address is not None,
Comment on lines +514 to +518

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve saved-list fields when replacing duplicates

Because this score is used to overwrite a duplicate Place wholesale, alias-dedupe cases where the earlier row contains saved-list metadata such as note, is_favorite, or added_by and the later canonical row only has better IDs will silently discard those user fields. I reproduced this by giving the first alias row Northwind's note/favorite and clearing them on the canonical row; the parsed canonical place lost both, so the replacement should merge preserved fields or account for them before overwriting.

Useful? React with 👍 / 👎.

)


def _find_place_metadata(ancestors: Sequence[JSONValue]) -> list[JSONValue] | None:
place_record = _find_place_record(ancestors)
metadata_node = _place_metadata_from_record(place_record)
Expand Down Expand Up @@ -597,6 +677,19 @@ def _find_cid(node: list[JSONValue] | None) -> str | None:
return None


def _find_cid_aliases(node: list[JSONValue] | None) -> list[str]:
if node is None:
return []
structured_value = _safe_index(node, 6)
structured_cid = _find_cid_in_structured_slot(structured_value)
if structured_cid is None:
return []
return _find_cid_aliases_in_structured_slot(
structured_value,
selected_cid=structured_cid,
)


def _find_google_id(node: list[JSONValue] | None) -> str | None:
if node is None:
return None
Expand Down Expand Up @@ -660,6 +753,27 @@ def _find_cid_in_structured_slot(value: JSONValue | None) -> str | None:
return _normalize_cid_token(numeric_texts[0])


def _find_cid_aliases_in_structured_slot(
value: JSONValue | None,
*,
selected_cid: str,
) -> list[str]:
if not isinstance(value, list):
return []
owner = _parse_list_owner(value)
if owner is not None and (owner.photo_url is not None or owner.profile_id is not None):
return []
aliases: list[str] = []
for item in value:
text = _clean_text(item)
if text is None or _LONG_INTEGER_PATTERN.fullmatch(text) is None:
continue
alias = _normalize_cid_token(text)
if alias is not None and alias != selected_cid and alias not in aliases:
aliases.append(alias)
return aliases
Comment thread
coderabbitai[bot] marked this conversation as resolved.


def _find_cid_in_fallback_value(value: JSONValue | None) -> str | None:
if isinstance(value, int | str):
return _normalize_fallback_cid_token(value)
Expand Down
59 changes: 59 additions & 0 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ def test_uses_fingerprint_not_positive_s2_cell_from_slot_6(self) -> None:
parsed = parse_saved_list_artifacts(_LIST_URL, runtime_state=runtime_state)

self.assertEqual(parsed.places[0].cid, _NORTHWIND_CID)
self.assertEqual(parsed.places[0].cid_aliases, [_NORTHWIND_CELL_ID])
self.assertNotEqual(parsed.places[0].cid, _NORTHWIND_CELL_ID)

def test_normalizes_negative_slot_6_fingerprint(self) -> None:
Expand Down Expand Up @@ -332,6 +333,64 @@ def test_dedupes_places_with_same_cid(self) -> None:
self.assertEqual(parsed.places[0].cid, _NORTHWIND_CID)
self.assertEqual(parsed.places[1].cid, _HARBOR_CID)

def test_dedupes_places_with_cid_alias(self) -> None:
runtime_state = copy.deepcopy(["noise", _LIST_NODE])
original_metadata = runtime_state[1][8][0][1]
assert isinstance(original_metadata, list)
original_metadata[7] = None
duplicate_place = copy.deepcopy(runtime_state[1][8][0])
duplicate_metadata = duplicate_place[1]
assert isinstance(duplicate_metadata, list)
duplicate_metadata[6] = [_NORTHWIND_CELL_ID]
duplicate_metadata[7] = None
runtime_state[1][8].append(duplicate_place)

parsed = parse_saved_list_artifacts(_LIST_URL, runtime_state=runtime_state)

self.assertEqual(len(parsed.places), 2)
self.assertEqual(parsed.places[0].cid, _NORTHWIND_CID)
self.assertEqual(parsed.places[1].cid, _HARBOR_CID)

Comment thread
coderabbitai[bot] marked this conversation as resolved.
def test_prefers_canonical_place_when_cid_alias_duplicate_appears_first(self) -> None:
runtime_state = copy.deepcopy(["noise", _LIST_NODE])
duplicate_place = copy.deepcopy(runtime_state[1][8][0])
duplicate_metadata = duplicate_place[1]
assert isinstance(duplicate_metadata, list)
duplicate_metadata[6] = [_NORTHWIND_CELL_ID]
duplicate_metadata[7] = None
runtime_state[1][8].insert(0, duplicate_place)

parsed = parse_saved_list_artifacts(_LIST_URL, runtime_state=runtime_state)

self.assertEqual(len(parsed.places), 2)
self.assertEqual(parsed.places[0].cid, _NORTHWIND_CID)
self.assertEqual(parsed.places[0].cid_aliases, [_NORTHWIND_CELL_ID])
self.assertEqual(parsed.places[0].google_id, "/g/11northwind")
self.assertEqual(parsed.places[1].cid, _HARBOR_CID)

def test_keeps_distinct_places_with_same_cid_alias_and_coordinates(self) -> None:
runtime_state = copy.deepcopy(["noise", _LIST_NODE])
first_place = runtime_state[1][8][0]
second_place = runtime_state[1][8][1]
assert isinstance(first_place, list)
assert isinstance(second_place, list)
first_metadata = first_place[1]
second_metadata = second_place[1]
assert isinstance(first_metadata, list)
assert isinstance(second_metadata, list)
first_metadata[7] = None
second_metadata[5] = [None, None, 35.6501307, 139.6868459]
second_metadata[6] = [_NORTHWIND_CELL_ID, _HARBOR_CID]
second_metadata[7] = None

parsed = parse_saved_list_artifacts(_LIST_URL, runtime_state=runtime_state)

self.assertEqual(len(parsed.places), 2)
self.assertEqual(parsed.places[0].cid, _NORTHWIND_CID)
self.assertEqual(parsed.places[1].cid, _HARBOR_CID)
self.assertEqual(parsed.places[0].cid_aliases, [_NORTHWIND_CELL_ID])
self.assertEqual(parsed.places[1].cid_aliases, [_NORTHWIND_CELL_ID])

def test_keeps_distinct_places_that_share_a_cid(self) -> None:
runtime_state = copy.deepcopy(["noise", _LIST_NODE])
second_place = runtime_state[1][8][1]
Expand Down