Emit CID aliases from parser#50
Conversation
📝 WalkthroughWalkthroughAdds a ChangesCID alias deduplication
Estimated code review effort: 2 (Simple) | ~15 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 |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e3a6fd75-ccc9-4cf3-89df-72f0d4ffa476) |
![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: 818c50db8e
ℹ️ 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".
| lng: float | ||
| maps_url: str | ||
| cid: str | None = None | ||
| cid_aliases: list[str] = field(default_factory=list) |
There was a problem hiding this comment.
Because Place is exported as a public model, inserting cid_aliases before google_id changes the generated dataclass __init__ positional order. A caller that still passes the old positional tail (cid, google_id, is_favorite, added_by) will now put the Google ID string into cid_aliases and shift the remaining values, so saved-list serialization is corrupted. Please append the new optional field after the existing fields, or make construction keyword-only, to keep the existing positional ABI.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_770d18c9-33c4-429a-a60b-93897d168d07) |
![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
🤖 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 `@src/gmaps_scraper/parser.py`:
- Around line 683-701: Add a None check in _find_cid_aliases_in_structured_slot
before appending the normalized CID token: _normalize_cid_token can return None,
so ensure alias is not None and still differs from selected_cid before calling
aliases.append. Keep the fix localized in this helper so the aliases list
remains list[str] and the type checker can verify the flow.
In `@tests/test_parser.py`:
- Around line 336-349: The test in test_dedupes_places_with_cid_alias is not
exercising the cid_aliases dedup path because the copied place still has a
google_id that triggers the earlier gid-based branch in
parse_saved_list_artifacts. Update the fixture setup so both the original and
duplicate place have google_id removed or nulled, forcing the parser to
deduplicate via the CID alias intersection logic instead. Keep the assertions on
parsed.places the same, and use the existing symbols parse_saved_list_artifacts,
_LIST_NODE, and duplicate_place to make the intent clear.
🪄 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: c03d6e6c-8516-4be1-abed-595c343453fe
📒 Files selected for processing (3)
src/gmaps_scraper/models.pysrc/gmaps_scraper/parser.pytests/test_parser.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df51b9eab7
ℹ️ 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".
| elif cid is not None: | ||
| dedupe_keys = { | ||
| f"cid:{cid_value}:{lat:.6f}:{lng:.6f}" | ||
| for cid_value in [cid, *cid_aliases] |
There was a problem hiding this comment.
When two different saved places share the same exact coordinates/S2 cell, such as two venues in one building, adding every cid_aliases value to the dedupe key makes the later place collide on the shared positive S2-cell alias and be skipped even when its true CID differs. Because the alias can be the positive S2-cell token rather than a unique place CID, this regresses the previous cid+lat/lng behavior that preserved distinct CIDs at the same location.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_caa099da-c2bb-40e7-9123-31cfdd24918b) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc08068fda
ℹ️ 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".
| if seen.intersection(dedupe_keys): | ||
| continue |
There was a problem hiding this comment.
When an alias-only duplicate appears before the full place row, this intersection check skips the later canonical row instead of replacing/merging it, so the output keeps the S2-cell alias as cid and drops the real fingerprint and google_id. I reproduced this by inserting the new test_dedupes_places_with_cid_alias duplicate before the original fixture row; the parser returned Northwind Cafe with cid=7451636382641713350, google_id=None, and no aliases, losing the corrected CID this change is meant to preserve.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c0a81ec7-29d7-442f-a924-34878238de78) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb929fdc41
ℹ️ 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".
| 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 |
There was a problem hiding this comment.
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 👍 / 👎.
| return ( | ||
| place.google_id is not None, | ||
| bool(place.cid_aliases), | ||
| place.cid is not None, | ||
| place.address is not None, |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Summary
Tests
Note
Low Risk
Changes are confined to saved-list parsing and serialization; behavior is covered by new unit tests with no auth or external API impact.
Overview
Parsed places now carry optional
cid_aliases(serialized when non-empty) so alternate structured CID tokens—e.g. S2 cell IDs alongside the chosen fingerprint—remain visible after parser fixes.Parsing collects other long-integer tokens from metadata slot 6 (excluding the selected CID). Deduping no longer relies on a single key: it indexes primary keys (
google_id,cid+coords, or name+coords) and alias keys, merges duplicates when any key collides, and replaces the kept row when a later duplicate has richer metadata (google id, aliases, cid, address).Tests cover alias emission, alias-based dedupe, preferring the canonical record when an alias-only duplicate appears first, and keeping two places that share an alias but differ by coordinates.
Reviewed by Cursor Bugbot for commit eb929fd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes