Repository navigation
Conversation
📝 WalkthroughWalkthroughThis PR refines Kepler dataset synchronization by streamlining the sync logic and updating metadata handling. The ChangesKepler Dataset Sync Refinement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/kepler/src/KeplerSlice.ts (1)
653-662:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReintroduce identifier validation before syncing missing datasets.
At Line 658,
dataIdis used as a table identifier without a validity/existence gate. Since these IDs come from map config references, non-table or malformed IDs will now hit DB query construction/execution on every sync pass and fail repeatedly. Add a guard beforeaddTableToMap(or validate insideaddTableToMap) so only resolvable table identifiers are queried.Suggested fix
for (const dataId of referencedDataIds) { if (keplerDatasets?.[dataId]) { continue; } + const tableExists = Boolean(get().db.tables?.[dataId]); + if (!tableExists) { + continue; + } try { await get().kepler.addTableToMap(mapId, dataId, { autoCreateLayers: false, centerMap: false, }); } catch (e) {🤖 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 `@packages/kepler/src/KeplerSlice.ts` around lines 653 - 662, The loop over referencedDataIds calls get().kepler.addTableToMap(mapId, dataId, ...) without validating dataId; add a pre-check to only attempt sync for resolvable table identifiers (e.g., verify format and existence via your table registry or kepler dataset index) before calling addTableToMap. Specifically, in the block that iterates referencedDataIds, guard with a predicate like "isValidTableId(dataId)" or "knownTables.has(dataId)" (or add the same guard inside addTableToMap) so malformed/non-table IDs are skipped and never used to construct DB queries; keep the existing keplerDatasets check and apply the new identifier validation immediately before the call to get().kepler.addTableToMap(mapId, dataId, {...}).
🤖 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.
Outside diff comments:
In `@packages/kepler/src/KeplerSlice.ts`:
- Around line 653-662: The loop over referencedDataIds calls
get().kepler.addTableToMap(mapId, dataId, ...) without validating dataId; add a
pre-check to only attempt sync for resolvable table identifiers (e.g., verify
format and existence via your table registry or kepler dataset index) before
calling addTableToMap. Specifically, in the block that iterates
referencedDataIds, guard with a predicate like "isValidTableId(dataId)" or
"knownTables.has(dataId)" (or add the same guard inside addTableToMap) so
malformed/non-table IDs are skipped and never used to construct DB queries; keep
the existing keplerDatasets check and apply the new identifier validation
immediately before the call to get().kepler.addTableToMap(mapId, dataId, {...}).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6aeff8c0-101c-4aec-8cc5-7e43029bdbcc
📒 Files selected for processing (1)
packages/kepler/src/KeplerSlice.ts
ilyabo
changed the title
Summary by CodeRabbit
Bug Fixes
Chores