-
-
Notifications
You must be signed in to change notification settings - Fork 3
Emit CID aliases from parser #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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.""" | ||
|
|
||
|
|
@@ -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): | ||
|
|
@@ -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) | ||
|
|
@@ -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 | ||
| _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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because this score is used to overwrite a duplicate 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) | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
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) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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_indexsees 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 👍 / 👎.