Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramssequenceDiagram
participant App as App / Room
participant Store as Room Store (Cesium slice)
participant Viewer as Cesium Viewer
participant DuckDB as DuckDB Connector
participant EntityLayer as Cesium Entity Layer
participant Clock as Clock Sync
App->>Store: initialize roomStore with Cesium slice
Store->>Viewer: create/register Viewer instance
Viewer->>Store: set viewer, apply camera config
Store->>Clock: apply persisted clock config
Clock->>Viewer: set start/stop/current time & multiplier
Store->>EntityLayer: register sql-entities layer (earthquakes)
EntityLayer->>DuckDB: execute SQL query
DuckDB-->>EntityLayer: return rows
EntityLayer->>Viewer: create Cesium entities (positions, availability)
Viewer->>Clock: onTick events
Clock->>Store: update currentTime in store
sequenceDiagram
participant User as User
participant Toolbar as Cesium Toolbar
participant Store as Cesium Store
participant Viewer as Cesium Viewer
participant Camera as Camera Listener
User->>Toolbar: click zoom-to-fit
Toolbar->>Store: call zoomToFit()
Store->>Viewer: compute bounds and flyTo()
Viewer->>Camera: camera moveEnd
Camera->>Store: saveCameraPosition()
Note over Store: persist camera to config
User->>Toolbar: toggle layer visibility
Toolbar->>Store: toggleLayerVisibility(id)
Store->>Viewer: re-render layers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
examples/cesium-app/.gitignore (1)
27-27: Broaden env ignore pattern to reduce secret-leak risk.
Only.envis ignored currently;.env.development,.env.production, etc. can still be committed accidentally.Suggested update
# Environment .env +.env.* +!.env.example🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/.gitignore` at line 27, The .gitignore currently only ignores a single filename ".env", which still allows variants like ".env.development" or ".env.production" to be committed; update the .gitignore entry for environment files to a broader pattern (e.g., ".env*" or ".env.*") so all environment file variants are ignored, and ensure any specific safe exceptions are handled with explicit negation if needed (refer to the existing ".env" entry to replace/expand it).examples/cesium-app/.env.example (1)
1-3: Add a token-scope safety note.
This is good onboarding, but add a brief note to create a least-privilege/domain-restricted Ion token to reduce accidental quota abuse.Suggested update
# Cesium Ion Access Token # Get your free token at https://ion.cesium.com/tokens +# Use a token scoped to this app/domain with minimal permissions. VITE_CESIUM_ION_TOKEN=your_token_here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/.env.example` around lines 1 - 3, Update the .env.example to include a short safety note about token scope next to VITE_CESIUM_ION_TOKEN: instruct users to create a least-privilege Cesium Ion token and, if possible, restrict it to the specific domain or usage to avoid accidental quota abuse; reference the environment variable name VITE_CESIUM_ION_TOKEN in the note so it's clear which token to scope and restrict.examples/cesium-app/src/Room.tsx (1)
23-27: Optional: tie selected state to modal visibility.Using a constant
falsemakes the sidebar button state static; consider bindingisSelectedtosqlEditorDisclosure.isOpenfor consistent UI feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/src/Room.tsx` around lines 23 - 27, The SidebarButton's selected state is hard-coded to false; update the isSelected prop on RoomShell.SidebarButton to reflect the modal visibility by using sqlEditorDisclosure.isOpen instead of false so the button visually reflects the SQL editor's open/closed state (keep onClick as sqlEditorDisclosure.onToggle).examples/cesium-app/package.json (1)
7-9: Consider enforcing workspace build beforedev.Given the workspace-linked dependencies, add a
predevstep so the example starts reliably from a clean checkout.♻️ Suggested script tweak
"scripts": { - "dev": "vite", + "predev": "pnpm -w build", + "dev": "vite", "build": "tsc && vite build", "preview": "vite preview" },Based on learnings: "Always run
pnpm buildfirst before running example apps, as example apps depend on builtsqlrooms/*packages viaworkspace:*links".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/package.json` around lines 7 - 9, Add a predev script that runs the workspace build before the existing dev script so examples with workspace:* links are built from a clean checkout; specifically, update package.json scripts to add a "predev" entry that invokes the workspace build command (e.g., pnpm -w -r build) so npm/pnpm will run it automatically before the "dev" script defined alongside "build" and "preview".packages/cesium/src/components/CesiumViewerWrapper.tsx (1)
185-196: Unsupported layer types silently produce no output.Layers with
type: 'geojson' | 'czml' | 'tileset'are defined inCesiumLayerConfigbut returnnullhere. Users configuring these layer types will see no error and no rendered content.Consider adding a console warning or a placeholder component for unsupported types to aid debugging.
💡 Add warning for unsupported layer types
switch (layer.type) { case 'sql-entities': return <CesiumEntityLayer key={layer.id} layerConfig={layer} />; // Future layer types: geojson, czml, tileset default: + if (process.env.NODE_ENV === 'development') { + console.warn( + `[CesiumViewerWrapper] Layer type "${layer.type}" not yet implemented: ${layer.id}` + ); + } return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cesium/src/components/CesiumViewerWrapper.tsx` around lines 185 - 196, The render loop in CesiumViewerWrapper (layers.map -> switch on layer.type) silently returns null for supported-but-unhandled types ('geojson', 'czml', 'tileset'); update the switch default to surface the issue by either logging a warning (console.warn or app logger) that includes layer.id and layer.type and/or rendering a small placeholder component (e.g., UnsupportedLayerPlaceholder) so developers see feedback instead of silent failure; be sure to reference the existing map callback and return a valid React node for unsupported types rather than null.packages/cesium/README.md (1)
65-78: Consider immutable pattern for layer configuration example.The example mutates
cesiumConfig.cesium.layersdirectly with assignment. While this works during initial setup, using spread syntax would better demonstrate immutable patterns consistent with Zustand/Immer usage elsewhere in the codebase.📝 Immutable config example
const cesiumConfig = createDefaultCesiumConfig(); -cesiumConfig.cesium.layers = [ - { - id: 'my-data', - type: 'sql-entities', - visible: true, - sqlQuery: 'SELECT longitude, latitude, name FROM my_table', - columnMapping: { - longitude: 'longitude', - latitude: 'latitude', - label: 'name', - }, - }, -]; +cesiumConfig.cesium = { + ...cesiumConfig.cesium, + layers: [ + { + id: 'my-data', + type: 'sql-entities', + visible: true, + sqlQuery: 'SELECT longitude, latitude, name FROM my_table', + columnMapping: { + longitude: 'longitude', + latitude: 'latitude', + label: 'name', + }, + }, + ], +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cesium/README.md` around lines 65 - 78, The example mutates the default config by assigning to cesiumConfig.cesium.layers; instead, construct a new config object using an immutable update (e.g., spread) so you don't mutate the object returned by createDefaultCesiumConfig; update the snippet to create a new cesiumConfig value that preserves existing properties and sets cesium.layers to a new array (referencing createDefaultCesiumConfig and the cesiumConfig.cesium.layers property names so reviewers can locate and replace the direct assignment).packages/cesium/src/hooks/useClockSync.ts (1)
81-83: Potential feedback loop between clock sync effects.The selector at line 82 returns the entire
clockConfigobject which includescurrentTime. When the tick handler (line 98) callssetCurrentTime, it updatesconfig.clock.currentTime, causingclockConfigto change and triggering the effect at lines 112-115. This reapplies the full config back to Cesium, including the currentTime that was just read from Cesium.While the 500ms throttle mitigates performance impact, consider selecting only the config properties that should flow store→Cesium (excluding
currentTime) to avoid redundant writes.♻️ Separate selectors to break feedback loop
export function useClockSync(): void { const viewer = useStoreWithCesium((s) => s.cesium.viewer); - const clockConfig = useStoreWithCesium((s) => s.cesium.config.clock); + // Select individual config properties to avoid feedback loop with currentTime + const multiplier = useStoreWithCesium((s) => s.cesium.config.clock.multiplier); + const shouldAnimate = useStoreWithCesium((s) => s.cesium.config.clock.shouldAnimate); + const clockRange = useStoreWithCesium((s) => s.cesium.config.clock.clockRange); const setCurrentTime = useStoreWithCesium((s) => s.cesium.setCurrentTime); // ... existing code ... // Store → Cesium: Apply config changes (excluding currentTime which flows Cesium→Store) useEffect(() => { if (!viewer) return; - applyClockConfigToViewer(viewer, clockConfig); - }, [viewer, clockConfig]); + const clock = viewer.clock; + clock.multiplier = multiplier; + clock.shouldAnimate = shouldAnimate; + clock.clockRange = CLOCK_RANGE_MAP[clockRange]; + }, [viewer, multiplier, shouldAnimate, clockRange]);Also applies to: 112-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cesium/src/hooks/useClockSync.ts` around lines 81 - 83, The effect is re-triggered because the selector (clockConfig from useStoreWithCesium) includes currentTime which the tick handler (setCurrentTime) writes back; change the selector used in useClockSync to return only the clock config fields that should flow store→Cesium (exclude currentTime) — e.g., replace the clockConfig selector with one that picks specific properties (intervals, multiplier, start/stop, etc.) or create clockConfigWithoutCurrentTime, then update the effect that reapplies config (the block referencing clockConfig at lines around the apply/reapply to Cesium) to use that trimmed selector so setCurrentTime updates no longer cause the effect to run and redundantly write currentTime back to Cesium.packages/cesium/src/components/CesiumClock.tsx (1)
28-40: Consider defensive guard for non-positive multiplier.
Math.log(multiplier)on line 33 returns-Infinityfor 0 andNaNfor negative values. While the current UI only sets positive values fromSPEED_STEPS, a defensive guard would prevent unexpected behavior if the store ever receives invalid data from persistence or external code.🛡️ Optional defensive fix
function multiplierToSlider(multiplier: number): number { + // Guard against invalid multiplier values + const safeMultiplier = multiplier > 0 ? multiplier : 1; let closest = 0; let minDiff = Infinity; for (let i = 0; i < SPEED_STEPS.length; i++) { const step = SPEED_STEPS[i]!; - const diff = Math.abs(Math.log(multiplier) - Math.log(step.value)); + const diff = Math.abs(Math.log(safeMultiplier) - Math.log(step.value)); if (diff < minDiff) { minDiff = diff; closest = i; } } return closest; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cesium/src/components/CesiumClock.tsx` around lines 28 - 40, The multiplierToSlider function calls Math.log(multiplier) which yields -Infinity/NaN for non-positive inputs; add a defensive guard at the start of multiplierToSlider to handle multiplier <= 0 (e.g., return index 0 or clamp to the smallest positive SPEED_STEPS value) and also ensure you don't call Math.log on any SPEED_STEPS entries that could ever be non-positive; update multiplierToSlider to early-return a safe default (or clamp) when multiplier is invalid and proceed with the existing nearest-step logic using Math.log only on known positive values.packages/cesium/src/cesium-types.ts (1)
37-47: Prefer composingCesiumRuntimeStateto avoid type drift.
CesiumSliceState.cesiumduplicates runtime fields already defined inCesiumRuntimeState; composing them would reduce maintenance risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cesium/src/cesium-types.ts` around lines 37 - 47, CesiumSliceState currently re-declares runtime fields that already exist on CesiumRuntimeState; update CesiumSliceState.cesium to compose the runtime type instead of duplicating fields by replacing the inline runtime properties with a reference to CesiumRuntimeState (e.g. have cesium: { config: CesiumSliceConfig['cesium']; } & CesiumRuntimeState or cesium: { config: CesiumSliceConfig['cesium']; runtime: CesiumRuntimeState } depending on the intended shape), ensuring you keep the persisted config (CesiumSliceConfig['cesium']) and remove duplicated fields like viewer, isAnimating, selectedEntity, isLoadingData to avoid type drift and maintenance duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/cesium-app/README.md`:
- Line 8: The README's "Earthquake Data" bullet incorrectly says "magnitude
4.0+" but the code in src/store.ts filters with "WHERE Magnitude >= 5.0"; update
the README bullet for "Earthquake Data" to state "magnitude 5.0+" so the
documentation matches the actual filter in src/store.ts (look for the WHERE
Magnitude >= 5.0 clause or any constant used for the magnitude threshold).
- Line 45: The README's "Speed slider" range is incorrect; update the text to
match the actual options implemented in CesiumClock.tsx by referencing the
SPEED_STEPS constant (which yields speeds from 1x up to 31,536,000x, i.e., 1
year per second). Replace "0.1x to 100x" with a concise description that
reflects SPEED_STEPS (for example "1x to 31,536,000x (up to 1 year per second)")
or otherwise describe the slider as using the discrete SPEED_STEPS values
defined in CesiumClock.tsx.
In `@examples/cesium-app/src/index.css`:
- Around line 5-8: Update the Stylelint configuration so Tailwind v4 `@source`
directives are treated as known at-rules: in .stylelintrc.json (which currently
extends "stylelint-config-standard-scss") add/modify the
"scss/at-rule-no-unknown" rule to include an ignoreAtRules array containing
"source" and "apply" so the linter no longer flags Tailwind's `@source` directives
in examples/cesium-app/src/index.css as unknown.
In `@examples/cesium-app/src/Room.tsx`:
- Around line 16-18: The useEffect invoking roomStore.getState().initialize can
run twice under React 18 StrictMode; make initialization idempotent by guarding
it before calling initialize — e.g., check a persisted/in-memory flag on the
store state (like roomStore.getState().initialized) or add an initializeOnce
guard inside the initialize function so repeated calls do nothing; update the
call site where roomStore.getState().initialize is invoked (and/or the
initialize method itself) to return early if already initialized so database
setup/data download runs only once.
In `@packages/cesium/src/cesium-config.ts`:
- Around line 12-19: The file-level comment above the CameraPosition schema
incorrectly states "All angles in radians" while the CameraPosition z.object and
its fields longitude and latitude are documented and defaulted in degrees;
update the documentation to accurately reflect degrees (e.g., change the header
to "All angles in degrees" or otherwise clarify units) so CameraPosition,
longitude, and latitude consistently advertise degrees to API consumers.
- Around line 174-175: createDefaultCesiumConfig currently returns the shared
DEFAULT_CESIUM_CONFIG object which allows callers to accidentally mutate a
global singleton; change createDefaultCesiumConfig to return a fresh copy of
DEFAULT_CESIUM_CONFIG (e.g., deep-clone via structuredClone or other
project-approved deep copy utility) so each call produces an independent
CesiumSliceConfig instance and no mutations leak across callers.
In `@packages/cesium/src/components/CesiumEntityLayer.tsx`:
- Around line 89-94: The debug console.log in CesiumEntityLayer that prints
clock range and entity metadata should be removed from production: locate the
console.log call in the CesiumEntityLayer component (the block that logs
startTime, stopTime, entityCount, hasAvailability when setting the clock range)
and either delete it or wrap it behind a conditional debug check (e.g., only run
when a dev/debug flag or NODE_ENV !== 'production') or replace it with the
project's logger at debug level; ensure no plain console.log remains in the
production build.
In `@packages/cesium/src/hooks/useSqlToCesiumEntities.ts`:
- Around line 75-105: The map over rows builds Cesium entities without
validating coordinates or time; guard against NaN/invalid values by validating
lon/lat/alt are finite numbers and timeStr is a valid ISO before calling
Cartesian3.fromDegrees or JulianDate.fromIso8601. In the rows.map block
(rows.map, mapping.longitude, mapping.latitude, mapping.altitude,
Cartesian3.fromDegrees, mapping.time, toIso8601, JulianDate.fromIso8601) skip or
filter out rows where Number(...) yields NaN or Infinity, and only build
availability when the ISO/time parse succeeded (catch remains fine), otherwise
return null/undefined and filter those out before returning the final entity
array. Ensure label/color/size conversions only run for rows that passed the
coordinate/time checks.
---
Nitpick comments:
In `@examples/cesium-app/.env.example`:
- Around line 1-3: Update the .env.example to include a short safety note about
token scope next to VITE_CESIUM_ION_TOKEN: instruct users to create a
least-privilege Cesium Ion token and, if possible, restrict it to the specific
domain or usage to avoid accidental quota abuse; reference the environment
variable name VITE_CESIUM_ION_TOKEN in the note so it's clear which token to
scope and restrict.
In `@examples/cesium-app/.gitignore`:
- Line 27: The .gitignore currently only ignores a single filename ".env", which
still allows variants like ".env.development" or ".env.production" to be
committed; update the .gitignore entry for environment files to a broader
pattern (e.g., ".env*" or ".env.*") so all environment file variants are
ignored, and ensure any specific safe exceptions are handled with explicit
negation if needed (refer to the existing ".env" entry to replace/expand it).
In `@examples/cesium-app/package.json`:
- Around line 7-9: Add a predev script that runs the workspace build before the
existing dev script so examples with workspace:* links are built from a clean
checkout; specifically, update package.json scripts to add a "predev" entry that
invokes the workspace build command (e.g., pnpm -w -r build) so npm/pnpm will
run it automatically before the "dev" script defined alongside "build" and
"preview".
In `@examples/cesium-app/src/Room.tsx`:
- Around line 23-27: The SidebarButton's selected state is hard-coded to false;
update the isSelected prop on RoomShell.SidebarButton to reflect the modal
visibility by using sqlEditorDisclosure.isOpen instead of false so the button
visually reflects the SQL editor's open/closed state (keep onClick as
sqlEditorDisclosure.onToggle).
In `@packages/cesium/README.md`:
- Around line 65-78: The example mutates the default config by assigning to
cesiumConfig.cesium.layers; instead, construct a new config object using an
immutable update (e.g., spread) so you don't mutate the object returned by
createDefaultCesiumConfig; update the snippet to create a new cesiumConfig value
that preserves existing properties and sets cesium.layers to a new array
(referencing createDefaultCesiumConfig and the cesiumConfig.cesium.layers
property names so reviewers can locate and replace the direct assignment).
In `@packages/cesium/src/cesium-types.ts`:
- Around line 37-47: CesiumSliceState currently re-declares runtime fields that
already exist on CesiumRuntimeState; update CesiumSliceState.cesium to compose
the runtime type instead of duplicating fields by replacing the inline runtime
properties with a reference to CesiumRuntimeState (e.g. have cesium: { config:
CesiumSliceConfig['cesium']; } & CesiumRuntimeState or cesium: { config:
CesiumSliceConfig['cesium']; runtime: CesiumRuntimeState } depending on the
intended shape), ensuring you keep the persisted config
(CesiumSliceConfig['cesium']) and remove duplicated fields like viewer,
isAnimating, selectedEntity, isLoadingData to avoid type drift and maintenance
duplication.
In `@packages/cesium/src/components/CesiumClock.tsx`:
- Around line 28-40: The multiplierToSlider function calls Math.log(multiplier)
which yields -Infinity/NaN for non-positive inputs; add a defensive guard at the
start of multiplierToSlider to handle multiplier <= 0 (e.g., return index 0 or
clamp to the smallest positive SPEED_STEPS value) and also ensure you don't call
Math.log on any SPEED_STEPS entries that could ever be non-positive; update
multiplierToSlider to early-return a safe default (or clamp) when multiplier is
invalid and proceed with the existing nearest-step logic using Math.log only on
known positive values.
In `@packages/cesium/src/components/CesiumViewerWrapper.tsx`:
- Around line 185-196: The render loop in CesiumViewerWrapper (layers.map ->
switch on layer.type) silently returns null for supported-but-unhandled types
('geojson', 'czml', 'tileset'); update the switch default to surface the issue
by either logging a warning (console.warn or app logger) that includes layer.id
and layer.type and/or rendering a small placeholder component (e.g.,
UnsupportedLayerPlaceholder) so developers see feedback instead of silent
failure; be sure to reference the existing map callback and return a valid React
node for unsupported types rather than null.
In `@packages/cesium/src/hooks/useClockSync.ts`:
- Around line 81-83: The effect is re-triggered because the selector
(clockConfig from useStoreWithCesium) includes currentTime which the tick
handler (setCurrentTime) writes back; change the selector used in useClockSync
to return only the clock config fields that should flow store→Cesium (exclude
currentTime) — e.g., replace the clockConfig selector with one that picks
specific properties (intervals, multiplier, start/stop, etc.) or create
clockConfigWithoutCurrentTime, then update the effect that reapplies config (the
block referencing clockConfig at lines around the apply/reapply to Cesium) to
use that trimmed selector so setCurrentTime updates no longer cause the effect
to run and redundantly write currentTime back to Cesium.
🪄 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: b290a05d-c552-4370-b656-1e8142a97cb9
⛔ Files ignored due to path filters (2)
examples/cesium-app/public/sample-earthquakes.csvis excluded by!**/*.csvpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
examples/cesium-app/.env.exampleexamples/cesium-app/.gitignoreexamples/cesium-app/README.mdexamples/cesium-app/index.htmlexamples/cesium-app/package.jsonexamples/cesium-app/src/App.tsxexamples/cesium-app/src/Room.tsxexamples/cesium-app/src/index.cssexamples/cesium-app/src/main.tsxexamples/cesium-app/src/store.tsexamples/cesium-app/src/vite-env.d.tsexamples/cesium-app/tsconfig.jsonexamples/cesium-app/vite.config.tspackages/cesium/.gitignorepackages/cesium/README.mdpackages/cesium/eslint.config.jspackages/cesium/package.jsonpackages/cesium/src/cesium-config.tspackages/cesium/src/cesium-slice.tspackages/cesium/src/cesium-types.tspackages/cesium/src/components/CesiumClock.tsxpackages/cesium/src/components/CesiumEntityLayer.tsxpackages/cesium/src/components/CesiumPanel.tsxpackages/cesium/src/components/CesiumToolbar.tsxpackages/cesium/src/components/CesiumViewerWrapper.tsxpackages/cesium/src/hooks/useCesiumViewer.tspackages/cesium/src/hooks/useClockSync.tspackages/cesium/src/hooks/useSqlToCesiumEntities.tspackages/cesium/src/index.tspackages/cesium/src/utils.tspackages/cesium/tsconfig.json
| @source '../index.html'; | ||
| @source 'src/**/*.{ts,tsx}'; | ||
| @source '../../../packages/*/src/**/*.{ts,tsx}'; | ||
| @source '../node_modules/@sqlrooms/*/dist/'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find Stylelint config files:"
fd -HI 'stylelint|.stylelintrc' .
echo
echo "Inspect unknown at-rule configuration:"
rg -n --no-heading -C3 'at-rule-no-unknown|scss/at-rule-no-unknown|ignoreAtRules' .Repository: sqlrooms/sqlrooms
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Stylelint Configuration ==="
cat .stylelintrc.json
echo
echo "=== Target CSS File (lines 1-20) ==="
head -20 examples/cesium-app/src/index.css
echo
echo "=== Tailwind Version in Root ==="
jq '.devDependencies.tailwindcss // .dependencies.tailwindcss // "not found"' package.json
echo
echo "=== Tailwind Version in cesium-app ==="
jq '.devDependencies.tailwindcss // .dependencies.tailwindcss // "not found"' examples/cesium-app/package.json 2>/dev/null || echo "cesium-app package.json not found or no tailwindcss"Repository: sqlrooms/sqlrooms
Length of output: 1962
Configure Stylelint to accept Tailwind v4 @source directives.
The CSS file uses Tailwind v4's @source at-rules (lines 5-8), which are valid but not standard SCSS. The current Stylelint config (extends: "stylelint-config-standard-scss") will flag these as unknown at-rules via scss/at-rule-no-unknown.
Update .stylelintrc.json to ignore Tailwind at-rules:
{
"extends": ["stylelint-config-standard-scss"],
"rules": {
"scss/at-rule-no-unknown": ["error", { "ignoreAtRules": ["source", "apply"] }],
// ... rest of existing rules
}
}🧰 Tools
🪛 Stylelint (17.6.0)
[error] 5-5: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 6-6: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 7-7: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
[error] 8-8: Unexpected unknown at-rule "@source" (scss/at-rule-no-unknown)
(scss/at-rule-no-unknown)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/cesium-app/src/index.css` around lines 5 - 8, Update the Stylelint
configuration so Tailwind v4 `@source` directives are treated as known at-rules:
in .stylelintrc.json (which currently extends "stylelint-config-standard-scss")
add/modify the "scss/at-rule-no-unknown" rule to include an ignoreAtRules array
containing "source" and "apply" so the linter no longer flags Tailwind's `@source`
directives in examples/cesium-app/src/index.css as unknown.
|
Addressed CodeRabbit feedback across two commits:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cesium/src/hooks/useClockSync.ts`:
- Around line 86-95: The selector is creating a derived object inside
useStoreWithCesium (using useShallow) and omitted currentTime, breaking
store→Cesium updates (calls to setCurrentTime never update
viewer.clock.currentTime); refactor by selecting raw fields individually from
useStoreWithCesium (e.g., startTime, stopTime, multiplier, shouldAnimate,
clockRange, currentTime, and setCurrentTime) and use useMemo to group
non-primitive inputs if needed, then add a guarded effect that, when
store.currentTime changes and differs from viewer.clock.currentTime, writes
store.currentTime into viewer.clock.currentTime (and guard against feedback
loops by skipping when values are already equal or when the change originated
from the viewer), ensuring setCurrentTime calls propagate to
viewer.clock.currentTime.
In `@packages/cesium/src/hooks/useSqlToCesiumEntities.ts`:
- Around line 60-73: The code computes stopJd from the last row which assumes
rows are time-sorted; change it to derive stopJd from the maximum valid
timestamp across all rows when mapping.time exists. Iterate over rows, parse
each timestamp via toIso8601 and JulianDate.fromIso8601 (skip invalid parses),
track the latest JulianDate (compare with JulianDate.lessThan/greaterThan or
convert to seconds), and assign that to stopJd so availability uses the true max
time rather than rows[rows.length - 1].
🪄 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: 6082c45d-6cca-42cb-b87f-20f0224b622e
📒 Files selected for processing (6)
examples/cesium-app/README.mdexamples/cesium-app/src/Room.tsxpackages/cesium/src/cesium-config.tspackages/cesium/src/components/CesiumEntityLayer.tsxpackages/cesium/src/hooks/useClockSync.tspackages/cesium/src/hooks/useSqlToCesiumEntities.ts
✅ Files skipped from review due to trivial changes (1)
- examples/cesium-app/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/cesium-app/src/Room.tsx
- packages/cesium/src/components/CesiumEntityLayer.tsx
- packages/cesium/src/cesium-config.ts
|
Addressed the second round of CodeRabbit feedback in
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cesium/src/hooks/useSqlToCesiumEntities.ts`:
- Around line 121-128: The descriptor creation is coercing missing/invalid
mapped values into "undefined"/"null" strings and allowing NaN/Infinity for
size; update the descriptors.push logic in useSqlToCesiumEntities so you first
read const val = row[mapping.label] / row[mapping.color] / row[mapping.size],
then only set label/color if val !== null && val !== undefined, converting to
String only after that check, and for size convert to Number and assign only
when Number.isFinite(sizeVal); ensure mapping.label, mapping.color and
mapping.size are guarded this way before adding them to the descriptor to avoid
invalid values leaking into rendering.
🪄 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: a0edc728-433a-44d6-b19a-587c3f4e824b
📒 Files selected for processing (2)
packages/cesium/src/hooks/useClockSync.tspackages/cesium/src/hooks/useSqlToCesiumEntities.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cesium/src/hooks/useClockSync.ts
|
Addressed the remaining CodeRabbit comment in
|
|
Thanks for the contribution! I am not yet quite sure it's worth adding a dedicated package at this point (could you convince me otherwise? :) ), but an example would definitely be a valuable addition. In this PR I am trying to make the example a bit more interesting and a better showcase of what Cesium can do by using opensky flight data which can be nicely animated: 
what do you think? |
|
Significantly expanded the example based on feedback that the demo needed more visual impact. The Cesium example is now a Wadati–Benioff Explorer — a 3D globe cross-section tool for subduction zones. Four new commits:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
examples/cesium-app/src/HistogramPanel.tsx (1)
53-53: Prefer selecting a stable primitive over the table object.
findTableByName('earthquakes')returns an object (orundefined). If its identity isn't referentially stable across unrelated store updates, this selector can trigger extra re-renders of the panel. Matching the pattern already used inRenderingStatus.tsx, coerce to a boolean inside the selector so only readiness transitions re-render:♻️ Suggested change
- const table = useRoomStore((s) => s.db.findTableByName('earthquakes')); + const tableReady = useRoomStore((s) => + Boolean(s.db.findTableByName('earthquakes')), + ); const {data, isLoading, error} = useSql<HistogramRow>({ query: sqlQuery, - enabled: Boolean(table), + enabled: tableReady, });As per coding guidelines: "When using Zustand/room-store selectors in React, do not create derived arrays/objects/functions inside the selector ... Select stable raw state first, then derive with
React.useMemoin the component."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/src/HistogramPanel.tsx` at line 53, The selector currently returns the table object directly which may be referentially unstable; change the selector to pick a stable primitive (e.g., select the db: useRoomStore(s => s.db) or a boolean readiness flag like useRoomStore(s => Boolean(s.db.findTableByName('earthquakes')))), then derive the actual table object with React.useMemo inside the component (e.g., const table = React.useMemo(() => db.findTableByName('earthquakes'), [db]) or memoize based on the boolean flag) so useRoomStore, findTableByName and the table local variable are referenced but re-renders only occur on stable state changes.examples/cesium-app/src/store.ts (1)
73-91: Minor:'earthquakes'table name duplicated across files.
tableName: 'earthquakes'(line 78) and the data sourcetableName: 'earthquakes'(line 111) both duplicate theEARTHQUAKE_TABLEconstant that already exists inearthquake-slice.ts(currently not exported). Consider exporting and reusing it to keep the table identifier in one place; similarly,EARTHQUAKE_LAYER_IDis already imported here, so the pattern is consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/src/store.ts` around lines 73 - 91, Export the EARTHQUAKE_TABLE constant from earthquake-slice.ts and replace the hardcoded 'earthquakes' strings in store.ts (both the layers entry's tableName and the data source tableName) with that exported EARTHQUAKE_TABLE constant; update the import in store.ts (where EARTHQUAKE_LAYER_ID is imported) to also import EARTHQUAKE_TABLE so the table identifier is maintained in a single source (refer to symbols EARTHQUAKE_TABLE in earthquake-slice.ts and the layers.tableName / data source tableName in store.ts).examples/cesium-app/src/PresetPanel.tsx (1)
40-74:preset.cameraHeightis defined but never read.Every entry in
SUBDUCTION_PRESETSsetscameraHeight(e.g.900_000), and theSubductionPresetinterface documents it as "Camera height above ellipsoid at fly-to, in meters", butflyToPresetderives range fromslabDepthKminstead (line 62) andhandleClearuses a hard-coded18_000_000. Either wirecameraHeightinto the range computation or drop the field to avoid drift between preset data and actual behaviour.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/cesium-app/src/PresetPanel.tsx` around lines 40 - 74, The SubductionPreset.cameraHeight field is never used; update flyToPreset to prefer preset.cameraHeight (meters) as the camera range when present and otherwise fall back to the existing slab-derived range calculation, and likewise make handleClear use the same default/fallback rather than a hard-coded 18_000_000 so preset data and clear behavior stay consistent; touch the flyToPreset function (and the handleClear logic) to compute a rangeMeters = preset.cameraHeight ?? computedFallback and pass that into HeadingPitchRange, keeping existing conversions and fallbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/cesium-app/src/earthquake-presets.ts`:
- Around line 305-336: The docstring and implementation of buildSliceWhereClause
use a single-row cosine scale (cosLat computed from preset.latitude) while
SLICE_ALONG_SECTION_KM = 1500 allows ~13° latitude span, causing up to ~20–25%
edge scale error; fix by either (A) reducing SLICE_ALONG_SECTION_KM to a few
hundred km or (B) updating the docstring to explicitly warn about degradation
for large extents and add a TODO note referencing cosLat/preset.latitude to
revisit if large windows are needed; choose one of these and update references
to cosLat, SLICE_ALONG_SECTION_KM, and the function docstring accordingly so
code and comment are aligned.
In `@examples/cesium-app/src/store.ts`:
- Around line 10-14: Update the stale file-level JSDoc in
examples/cesium-app/src/store.ts (the top-of-file comment) to match the actual
viewer configuration (depthTestAgainstTerrain = true) and explain that because
depthTestAgainstTerrain is true subsurface events require a clipping plane to be
visible (match the inline comment near the viewer config around the existing
comment at lines ~66-69); make the same correction in
examples/cesium-app/src/earthquake-slice.ts (the file-level JSDoc around lines
~75-77) so both docstrings accurately state depthTestAgainstTerrain = true and
mention the clipping-plane requirement; keep the explanation brief and
consistent with the inline comments and references to the earthquake
slice/updateLayer behavior.
---
Nitpick comments:
In `@examples/cesium-app/src/HistogramPanel.tsx`:
- Line 53: The selector currently returns the table object directly which may be
referentially unstable; change the selector to pick a stable primitive (e.g.,
select the db: useRoomStore(s => s.db) or a boolean readiness flag like
useRoomStore(s => Boolean(s.db.findTableByName('earthquakes')))), then derive
the actual table object with React.useMemo inside the component (e.g., const
table = React.useMemo(() => db.findTableByName('earthquakes'), [db]) or memoize
based on the boolean flag) so useRoomStore, findTableByName and the table local
variable are referenced but re-renders only occur on stable state changes.
In `@examples/cesium-app/src/PresetPanel.tsx`:
- Around line 40-74: The SubductionPreset.cameraHeight field is never used;
update flyToPreset to prefer preset.cameraHeight (meters) as the camera range
when present and otherwise fall back to the existing slab-derived range
calculation, and likewise make handleClear use the same default/fallback rather
than a hard-coded 18_000_000 so preset data and clear behavior stay consistent;
touch the flyToPreset function (and the handleClear logic) to compute a
rangeMeters = preset.cameraHeight ?? computedFallback and pass that into
HeadingPitchRange, keeping existing conversions and fallbacks.
In `@examples/cesium-app/src/store.ts`:
- Around line 73-91: Export the EARTHQUAKE_TABLE constant from
earthquake-slice.ts and replace the hardcoded 'earthquakes' strings in store.ts
(both the layers entry's tableName and the data source tableName) with that
exported EARTHQUAKE_TABLE constant; update the import in store.ts (where
EARTHQUAKE_LAYER_ID is imported) to also import EARTHQUAKE_TABLE so the table
identifier is maintained in a single source (refer to symbols EARTHQUAKE_TABLE
in earthquake-slice.ts and the layers.tableName / data source tableName in
store.ts).
🪄 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: e5200953-6544-464b-b992-5e28620e911a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
examples/cesium-app/README.mdexamples/cesium-app/package.jsonexamples/cesium-app/scripts/fetch-earthquakes.pyexamples/cesium-app/src/HistogramPanel.tsxexamples/cesium-app/src/PresetPanel.tsxexamples/cesium-app/src/RenderingStatus.tsxexamples/cesium-app/src/Room.tsxexamples/cesium-app/src/SceneStyleController.tsxexamples/cesium-app/src/SlabSurfaceController.tsxexamples/cesium-app/src/SliceClippingController.tsxexamples/cesium-app/src/earthquake-presets.tsexamples/cesium-app/src/earthquake-slice.tsexamples/cesium-app/src/panel-keys.tsexamples/cesium-app/src/slab-mesh.tsexamples/cesium-app/src/store.tspackages/cesium/src/cesium-slice.tspackages/cesium/src/components/CesiumEntityLayer.tsx
✅ Files skipped from review due to trivial changes (3)
- examples/cesium-app/package.json
- examples/cesium-app/README.md
- examples/cesium-app/src/panel-keys.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cesium/src/components/CesiumEntityLayer.tsx
- packages/cesium/src/cesium-slice.ts
|
I updated my example to try to showcase why 3D matters for a general audience, still earthquakes though. I should be explicit that I'm personally interested in problems that involve the subsurface. Subsurface/3D spatial information systems is a nontrivial market that is addressed in very narrow domain-specific tools with typically very high per-seat costs. Examples are: support for tunnelling in the civil engineering domain (visualizing/modeling geotechnical data); visualization and modeling of subsurface environmental chemistry data in e.g. delineating a plume of a pollutant in groundwater; visualization and modeling of subsurface whole rock analytical or multispectral imaging data in a minerals exploration context; temperature in geothermal exploration; geophysics assets in all of the above, etc. etc. I am excited to have the real foundation for a frontend-heavy data-intensive experiments that duckdb-wasm and sqlrooms brings together. Thank you for your consideration.
|
|
Addressed the third CodeRabbit round in
|
|
Fair point — on reflection, a dedicated Cesium package isn't worth the monorepo's long-term maintenance cost for what this is right now. Pushed No behaviour change — the example still renders the Wadati–Benioff globe with subsurface seismicity, section-cut clipping, and the slab surface mesh. Build + typecheck green across the monorepo (41 packages, 45 typechecks). Re: the two examples — I don't have a strong opinion on which ships; happy for your opensky flight data branch (#534) to be the canonical Cesium example if you prefer. The quakes demo is there if you think the subsurface/clipping story adds something the flight demo doesn't, but no pressure. |
Adds examples/cesium-quakes-standalone, a Wadati-Benioff zone explorer built on CesiumJS + Resium. Six subduction-zone presets fly the camera to an along-strike view and cut the globe open with a clipping plane, revealing the dipping plane of seismicity that traces the descending slab from the trench down to ~700 km. The Cesium integration lives inline under src/cesium/ rather than as a workspace package, per review feedback on sqlrooms#530. The example depends only on packages already on main: duckdb, recharts, room-shell, sql-editor, ui. - SQL-driven entity layer: DuckDB query -> Cesium entities, with depth projected to altitude_m = -depth * 1000 so events sit at their true 3D position inside the Earth - Section-cut clipping plane + a translucent parametric slab surface tuned per-preset to Slab2.0 dip profiles - Live magnitude-vs-depth histogram tied to the active slice - Global M5+ catalog since 2013 (~17k events) loaded as parquet; scripts/fetch-earthquakes.py regenerates it from the USGS FDSN feed
Introduces @sqlrooms/cesium, a new package for 3D geospatial visualization built on CesiumJS and Resium, along with an example app demonstrating earthquake data visualization on a 3D globe.
Package features:
Example app (examples/cesium-app):
Summary by CodeRabbit