fix(cli): remove UI store cycles by ilyabo · Pull Request #811 · sqlrooms/sqlrooms

ilyabo · 2026-07-14T09:43:29Z

What changed

route CLI UI store reads through the existing RoomStateProvider seam instead of importing the concrete singleton store
move startup configuration and DuckDB runtime setup into leaf modules shared by the store and UI
inject imperative store access through useRoomStoreApi where callbacks need getState
extend the established Madge circular-dependency check to scan the CLI UI source tree

Why

store.ts imported artifact and layout registries whose renderers imported store.ts back. Those cycles caused top-level reads of experimentalEnabled to hit the JavaScript temporal dead zone during module initialization.

The new dependency direction keeps room.tsx as the composition root and lets UI renderers consume the store through the provider already mounted by RoomShell.

Validation

pnpm build
pnpm --filter sqlrooms-cli-app lint
pnpm --filter sqlrooms-cli-app test (33 tests)
pnpm --filter sqlrooms-cli-app build
pnpm check-circular-deps (1,062 files; package entry points and CLI app)
pre-push knip, workspace typecheck, circular dependency check, and full workspace tests
browser runtime verification for pnpm dev cli and pnpm dev cli --experimental; both rendered with zero JavaScript exceptions

Summary by CodeRabbit

New Features
- Added CLI runtime configuration with AI/experimental feature flags.
- Introduced shared CLI DuckDB WebSocket runtime with automatic spatial/spatial extension setup and Mosaic schema constants.
Bug Fixes
- Improved room-state access across chat, assistant, workspace, and worksheet interactions.
- Enhanced lost-connection reconnect flow and improved local file import behavior.
Chores
- Improved circular-dependency checks and adjusted linting/knip configuration.

coderabbitai · 2026-07-14T09:45:01Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c12b579-8324-471c-ad9d-fadbda4ae369

📥 Commits

Reviewing files that changed from the base of the PR and between 50d9400 and f536a76.

⛔ Files ignored due to path filters (1)

pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (2)

knip.json
package.json

📝 Walkthrough

Walkthrough

The CLI UI separates runtime configuration and DuckDB initialization, adds typed room-store hooks, and updates components to use local imperative store APIs where needed. Madge dependency-cycle checks are refined for TypeScript and TSX sources.

Changes

CLI runtime and room-store decoupling

Layer / File(s)	Summary
Runtime configuration and DuckDB modules `apps/sqlrooms-cli-ui/src/runtimeEnvironment.ts`, `apps/sqlrooms-cli-ui/src/duckDbRuntime.ts`, `apps/sqlrooms-cli-ui/src/store.ts`	Runtime configuration, feature flags, DuckDB connector initialization, and Mosaic schema constants are moved into dedicated runtime modules.
Room-store hook wrappers and reactive consumers `apps/sqlrooms-cli-ui/src/roomStoreHooks.ts`, `apps/sqlrooms-cli-ui/src/components/`, `apps/sqlrooms-cli-ui/src/workspace/`	Typed reactive and imperative room-store hooks are added, and reactive consumers import them from the new hooks module.
Imperative store access in workflows `apps/sqlrooms-cli-ui/src/components/AssistantChatContainer.tsx`, `apps/sqlrooms-cli-ui/src/components/CliDuckDbConnectionLostDialog.tsx`, `apps/sqlrooms-cli-ui/src/components/useLocalFileLoader.ts`, `apps/sqlrooms-cli-ui/src/workspace/AppBuilderArtifact.tsx`, `apps/sqlrooms-cli-ui/src/workspace/WorksheetArtifact.tsx`	Chat, reconnect, file loading, preview bridge, worksheet, and AI actions use the hook-provided store API and updated callback dependencies.
Runtime dependency-cycle validation `package.json`, `knip.json`	The Madge script and development tooling are updated to inspect TypeScript and TSX sources while excluding `dist` output and no longer ignoring Madge in Knip.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

sqlrooms/sqlrooms#360: Adds the underlying imperative room-store API used here.
sqlrooms/sqlrooms#585: Covers related DuckDB and Mosaic initialization changes.
sqlrooms/sqlrooms#763: Covers related reconnect-dialog behavior.

Suggested reviewers: dmitriy-kostianetskiy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: removing circular UI store dependencies in the CLI.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch agent/fix-cli-store-cycle

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

package.json (1)
23-23: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Use the pinned Madge binary. check-circular-deps runs npx madge, but madge isn’t declared in the workspace dependencies or lockfile, so this can fall back to a registry fetch. Switch to pnpm exec madge and add madge as a locked devDependency.
🤖 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 `@package.json` at line 23, Update the check-circular-deps script to invoke
Madge through pnpm exec instead of npx, and add Madge as a pinned workspace
devDependency with the lockfile updated accordingly.

🤖 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.

Nitpick comments:
In `@package.json`:
- Line 23: Update the check-circular-deps script to invoke Madge through pnpm
exec instead of npx, and add Madge as a pinned workspace devDependency with the
lockfile updated accordingly.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b2733e9-cef3-4fb5-8581-84a702078c63

📥 Commits

Reviewing files that changed from the base of the PR and between 38039d1 and 50d9400.

📒 Files selected for processing (1)

package.json

fix(cli): remove UI store cycles

38039d1

ilyabo requested a review from dmitriy-kostianetskiy July 14, 2026 09:45

chore: extend circular dependency check

50d9400

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

ilyabo added 2 commits July 14, 2026 13:26

chore: pin madge dependency

a5fbe59

chore: track madge usage with knip

f536a76

ilyabo merged commit d1eac98 into main Jul 14, 2026
6 checks passed

ilyabo deleted the agent/fix-cli-store-cycle branch July 14, 2026 14:03

coderabbitai Bot mentioned this pull request Aug 19, 2026

feat(cli): add named capability profiles #859

Merged

Navigation Menu

Repository navigation

fix(cli): remove UI store cycles - #811

fix(cli): remove UI store cycles#811
ilyabo merged 4 commits into
mainfrom
agent/fix-cli-store-cycle

ilyabo commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

ilyabo commented Jul 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

Validation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ilyabo commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading