chore: sync tables deletion when removed locally

[ChiragAgg5k]

Summary by CodeRabbit

• New Features
  • Push command now detects remote tables that no longer exist locally and prompts users to delete them
  • Displays a summary table of tables scheduled for deletion before proceeding
  • Deletion occurs only after explicit user confirmation

[coderabbitai]

Walkthrough

The change modifies the push command template to add table deletion detection and execution logic. When pushing table resources, the command now compares locally configured tables against remote tables, identifies remote tables that no longer exist locally, and presents a deletion plan to the user. Upon confirmation, the command deletes each remote table via the tables-db module. The implementation extends the tables-db module's public API by exporting two new functions: tablesDBListTables and tablesDBDeleteTable, which are imported and used within the push workflow.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The change introduces new feature logic (table deletion detection and confirmation flow) within a single file but requires understanding multiple integration points within the existing push workflow. The review must verify correct comparison of local versus remote tables, proper error handling during deletion operations, user confirmation flow, and integration sequencing within the broader push process. The API surface extension (new exports from tables-db) adds modest additional verification scope, but the contained scope to one file and the relatively straightforward deletion pattern keep this at moderate rather than complex level.

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 "chore: sync tables deletion when removed locally" directly aligns with the main change in the changeset. The implementation adds logic to detect and delete remote tables that no longer exist locally and integrates this deletion flow into the existing push workflow. The title is concise, clear, and uses specific terminology ("sync tables deletion", "removed locally") that meaningfully communicates the primary purpose of the change. A teammate reviewing the git history would immediately understand that this PR handles synchronization of table deletions when tables are removed locally.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sync-tables

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

templates/cli/lib/commands/push.js.twig (1)

254-261: Bug: wrong variable name used in deleteIndexes affects timeout scaling.

deleteIndexes(...) receives indexesKeys but computes steps using attributeKeys, which is undefined. This breaks adaptive timeout logic for index deletions.

Fix variable name:

-        if (pollMaxDebounces === POLL_DEFAULT_VALUE) {
-            let steps = Math.max(1, Math.ceil(attributeKeys.length / STEP_SIZE));
+        if (pollMaxDebounces === POLL_DEFAULT_VALUE) {
+            let steps = Math.max(1, Math.ceil(indexesKeys.length / STEP_SIZE));

🧹 Nitpick comments (1)

templates/cli/lib/commands/push.js.twig (1)

1888-1946: Don’t silently swallow non-404 errors when listing tables.

Current catch block skips all errors, which can hide auth/permission/API issues and give a false “No tables found to delete”. Log non-404s (at least under --verbose) so users can act.

Apply this diff to add diagnostic logging without being noisy:

@@
-    for (const db of localTablesDBs) {
+    for (const db of localTablesDBs) {
         try {
             const { tables: remoteTables } = await paginate(tablesDBListTables, {
                 databaseId: db.$id,
                 parseOutput: false
             }, 100, 'tables');
@@
-        } catch (e) {
-            // Skip if database doesn't exist or other errors
-        }
+        } catch (e) {
+            // Skip missing DBs but surface other issues
+            if (Number(e?.code) === 404) {
+                if (cliConfig.verbose) warn(`Skipping missing database ${db.name ?? db.$id}: ${e.message ?? e}`);
+            } else {
+                warn(`Failed to list tables for ${db.name ?? db.$id}. Skipping. ${e?.message ?? e}`);
+            }
+        }
     }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5970def and 746dcc0.

📒 Files selected for processing (1)

• templates/cli/lib/commands/push.js.twig (2 hunks)

⏰ 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). (12)

• GitHub Check: swift (server)
• GitHub Check: web (client)
• GitHub Check: android (client)
• GitHub Check: flutter (client)
• GitHub Check: build (8.3, Ruby27)
• GitHub Check: build (8.3, KotlinJava17)
• GitHub Check: build (8.3, Ruby30)
• GitHub Check: build (8.3, Node16)
• GitHub Check: build (8.3, KotlinJava8)
• GitHub Check: build (8.3, FlutterBeta)
• GitHub Check: build (8.3, Android14Java17)
• GitHub Check: build (8.3, Android5Java17)