Skip to content

Add: column waiting logic on suggestions as well#2663

Merged
ItzNotABug merged 1 commit intomainfrom
fix-sample-data
Dec 2, 2025
Merged

Add: column waiting logic on suggestions as well#2663
ItzNotABug merged 1 commit intomainfrom
fix-sample-data

Conversation

@ItzNotABug
Copy link
Copy Markdown
Contributor

@ItzNotABug ItzNotABug commented Dec 2, 2025

What does this PR do?

Fixes the issue of error when generating sample data right after ai suggested columns are generated. The state mayb not always be available for columns, so we wait.

Test Plan

Manual.

Related PRs and Issues

N/A.

Have you read the Contributing Guidelines on issues?

Yes.

Summary by CodeRabbit

  • New Features

    • Added real-time event tracking for column creation and updates in database operations with automatic monitoring and response capabilities.
  • Refactor

    • Refactored column observation logic into a dedicated, centralized utility module to enhance code reusability, maintainability, and consistency across multiple database management components.

✏️ Tip: You can customize this high-level summary in your review settings.

@ItzNotABug ItzNotABug self-assigned this Dec 2, 2025
@appwrite
Copy link
Copy Markdown

appwrite Bot commented Dec 2, 2025

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Every Git commit and branch gets its own deployment URL automatically

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 2, 2025

Walkthrough

This PR introduces a new setupColumnObserver module that centralizes logic for tracking column creation and updates via real-time events. The observer maintains column state, manages timeouts, and provides event handlers and promises for coordinating creation workflows. The module is then integrated into two Svelte components that previously contained inline column observation logic. The integration involves subscribing to real-time column events, wiring handlers, and using promise-based coordination to wait for column availability. Existing inline observation implementations are replaced with calls to the centralized module.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • setupColumnObserver logic: Verify promise resolution paths (timeout vs. count-based), proper state management with isActive flag, and timeout cleanup to prevent memory leaks
  • Real-time event subscription: Confirm correct event path patterns (databases.*.tables.*.columns.*) and handler attachment/detachment in both components
  • Async/await pattern changes: The onMount callback in empty.svelte changed from async to synchronous; verify that suggestColumns being called without await does not introduce race conditions or unintended concurrency issues
  • Cleanup and lifecycle: Ensure columnCreationHandler is properly reset in finally blocks and that the observer cleans up listeners after promises resolve or timeout
  • Handler coordination: Verify that the destructured startWaiting, waitPromise, and handler from setupColumnObserver are correctly wired and that the promise coordination works as intended in both integration points

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Add: column waiting logic on suggestions as well' clearly summarizes the main change: adding waiting logic for column state in the suggestions flow to handle timing issues with AI-suggested columns.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-sample-data

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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts (2)

29-33: Redundant clearTimeout call.

clearTimeout(timeout) is called on line 30, and then cleanup() on line 31 calls it again on line 40. While harmless, you could simplify by removing line 30 since cleanup() already handles it.

             if (expectedCount > 0 && availableColumns.size >= expectedCount) {
-                    clearTimeout(timeout);
                     cleanup();
                     resolvePromise();
                 }

52-56: Same redundant clearTimeout pattern.

Similar to above, line 53 clears the timeout before cleanup() is called, which also clears it.

         if (availableColumns.size >= expectedCount) {
-            clearTimeout(timeout);
             cleanup();
             resolvePromise();
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3b19df and 3b172e7.

📒 Files selected for processing (3)
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts (1 hunks)
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (7 hunks)
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
src/routes/**

📄 CodeRabbit inference engine (AGENTS.md)

Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
src/routes/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
🧠 Learnings (4)
📚 Learning: 2025-09-25T04:21:57.071Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2373
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:536-552
Timestamp: 2025-09-25T04:21:57.071Z
Learning: In the Appwrite console database suggestions flow, after successfully creating columns via `createColumns()`, the `await invalidate(Dependencies.TABLE)` call causes the view to be destroyed and another view (populated table view) to be rendered, automatically cleaning up component state without needing manual reset.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
📚 Learning: 2025-11-03T13:26:21.384Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2540
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/indexes/+page.svelte:100-108
Timestamp: 2025-11-03T13:26:21.384Z
Learning: In the Appwrite console, the waterfall/fake data generation flow does not create indexes, only columns and data. Therefore, index realtime listeners do not need to be gated with `!$isWaterfallFromFaker` like column update listeners.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte
📚 Learning: 2025-10-07T14:17:11.597Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts:28-28
Timestamp: 2025-10-07T14:17:11.597Z
Learning: In src/routes/(console)/project-[region]-[project]/databases/database-[database]/(entity)/helpers/terminology.ts, the empty `vectordb: {}` entry in baseTerminology is intentional. The vectordb database type is not currently used because the backend API is not finalized, and no database type returns 'vectordb' at the moment.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte
🧬 Code graph analysis (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts (2)
src/lib/stores/sdk.ts (1)
  • RealtimeResponse (214-219)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts (1)
  • Columns (9-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/+layout.svelte (1)

292-317: Clean integration of the external column observer.

The pattern of destructuring the observer's return values, assigning the handler, using startWaiting/waitPromise, and cleaning up in finally is well-structured. The finally block ensures columnCreationHandler is reset even if an error occurs, preventing stale handlers.

src/routes/(console)/project-[region]-[project]/databases/database-[database]/(observer)/columnObserver.ts (1)

59-64: Consider the cleanup export usage.

The cleanup function is exported but not called by current consumers. Both callers reset columnCreationHandler = null in their finally blocks but don't invoke cleanup. This works because:

  1. If the promise resolves (success or timeout), cleanup is called internally
  2. Setting the handler to null prevents further event processing

However, if a caller were to throw before startWaiting is called, the timeout would never be set, so there's nothing to clean up. The current design is acceptable.

src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte (2)

590-607: Good integration of real-time column observer.

The onMount correctly returns the realtime cleanup function, and the event filtering for databases.*.tables.*.columns.* is appropriate. Calling suggestColumns() without await is intentional to allow the real-time subscription to start immediately, and suggestColumns has its own error handling internally.


979-1111: Consistent observer integration in createColumns.

The pattern matches the implementation in +layout.svelte:

  1. Initialize observer and assign handler before creating columns
  2. Call startWaiting(count) after all creation requests complete
  3. Await waitPromise before invalidating and showing success notification
  4. Reset handler in finally block

This ensures columns are in available status before proceeding, fixing the race condition mentioned in the PR objectives.

@ItzNotABug ItzNotABug merged commit e977a00 into main Dec 2, 2025
4 checks passed
@ItzNotABug ItzNotABug deleted the fix-sample-data branch December 2, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants