Plan, progress, and TUI improvements for planetscale#37
Open
Plan, progress, and TUI improvements for planetscale#37
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes PlanetScale planning accuracy by selecting the correct “current schema” source based on whether safe schema changes are enabled, and it makes progress reporting keyspace-aware so duplicate table names across Vitess keyspaces are tracked and rendered correctly.
Changes:
- Choose PlanetScale plan “current schema” source based on branch safe-migrations mode (API when enabled, vtgate when disabled).
- Preserve and use namespace/keyspace in engine progress → task syncing → UI/API rendering to avoid collisions on duplicate table names.
- Add LocalScale
safe_migrationsconfig and Docker-backed integration tests covering both schema-source paths.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/tern/progress_keys.go | Adds shared (tern-local) namespace/table keying helpers for progress lookup. |
| pkg/tern/progress_keys_test.go | Unit tests ensuring namespace-aware progress matching and fallback behavior. |
| pkg/tern/local_control.go | Uses namespace-aware progress matching when snapshotting and applying engine progress to tasks. |
| pkg/tern/local_client.go | Uses namespace-aware engine progress mapping for terminal progress rendering and task selection tweaks. |
| pkg/tern/local_apply.go | Uses namespace-aware per-table progress when syncing atomic tasks from engine results. |
| pkg/localscale/testcontainer.go | Extends container DB config to allow toggling safe_migrations. |
| pkg/localscale/server_integration_test.go | Adds databases/keyspaces to test safe-migrations enabled/disabled behavior in containerized LocalScale. |
| pkg/localscale/server.go | Plumbs SafeMigrations config through to backend state with a default of true. |
| pkg/localscale/handlers_branches.go | Returns correct safe_migrations value in branch responses (including virtual main). |
| pkg/localscale/planetscale_engine_integration_test.go | Integration test validating plan schema source selection (API vs vtgate) and error behavior without DSN. |
| pkg/engine/engine.go | Adds Namespace field to engine.TableProgress for keyspace-aware progress. |
| pkg/engine/planetscale/planetscale.go | Adds schema-source selection logic and vtgate schema fetch implementation; includes namespace in aggregated progress. |
| pkg/api/progress_handlers.go | Makes API progress/task correlation namespace-aware and preserves keyspace in responses. |
| pkg/api/progress_handlers_test.go | Tests keyspace preservation for terminal applies and namespace-aware sync from Tern progress. |
| cmd/localscale/main.go | Adds safe_migrations to LocalScale config parsing and wiring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4e09f24 to
6342993
Compare
6342993 to
a9c1575
Compare
The TUI's footer switch had no case for the failed state, so apply errors from the progress API were never displayed. Now shows the error in red with recovery guidance when the apply reaches failed state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The engine already checks SafeMigrations from the PlanetScale branch response to decide whether to use the schema API or vtgate for plan. The api-plan environment with a broken DSN was a workaround to force the API path. Since LocalScale defaults safe_migrations to true, the staging environment already uses the schema API path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Schema fetching and Spirit PlanChanges diffing now run with up to 20 concurrent goroutines per keyspace. For databases with 30+ keyspaces this reduces plan time from sequential (30+ serial API calls) to parallel (~2s). Results are collected in deterministic keyspace order. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
be2e14a to
d20b978
Compare
When 6+ consecutive keyspaces have identical DDL changes, show the first 3 keyspace headers then "... and N more keyspaces with identical changes" followed by the DDL once. Common pattern for sharded databases with numbered keyspaces sharing the same schema. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ed06699 to
3d1216c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
safe_migrationsis enabled on the branch, vtgate when disabled. Removed theapi-planfake environment hack🤖 Generated with Claude Code