Add pkg/suggest: advisory safer-form rewrites with typed caveats

README.md

@@ -26,8 +26,8 @@ verdict when it can't prove one (see
 [docs/postgres-online-ddl-reference.md](docs/postgres-online-ddl-reference.md)).
 
 **Status: Phases 1 and 2.1–2.5.** The parse boundary, declarative diff,
-classifier, router seam, versioned dry-run plan report, and offline linter
-are implemented. `pg-sprite migrate --alter '…'` runs a bounded optimistic
+classifier, router seam, versioned dry-run plan report, offline linter, and
+advisory `suggest` command are implemented. `pg-sprite migrate --alter '…'` runs a bounded optimistic
 native attempt; routed execution beyond that attempt lands in Phase 3.
 Changes without an available backend get a structured refusal (exit code 2).
 The design docs and the phased

SAFETY.md

@@ -27,9 +27,9 @@ The invariant registry (invariant IDs referenced below) lives in
 | `pkg/checkpoint` — durable resume state | ✅ core | planned (Phase 8) | ST-1, ST-2 |
 | slot lifecycle (in `pkg/decode`) — create, reap, lag ceiling | ✅ core | planned (Phase 8) | ST-3 |
 | `pkg/migration` — orchestrator, **cutover swap + fidelity gate** | ✅ core | planned (Phase 7) | LK-2, LK-4, ST-5 |
-| `pkg/statement`, `pkg/planner`, `pkg/schemadiff`, `pkg/router`, `pkg/plan`, `pkg/lint` — classify/diff/route/report | ❌ periphery¹ | `pkg/statement` (parse boundary), `pkg/schemadiff` (introspect/diff via scratch execute-and-introspect), `pkg/planner` (classifier), `pkg/router` (backend assignment + availability policy), `pkg/plan` (versioned dry-run plan report), and `pkg/lint` (offline typed findings) exist (Phases 2.1–2.5) | (CO-7 holds at the parse boundary) |
+| `pkg/statement`, `pkg/planner`, `pkg/schemadiff`, `pkg/router`, `pkg/plan`, `pkg/lint`, `pkg/suggest` — classify/diff/route/report | ❌ periphery¹ | `pkg/statement` (parse boundary), `pkg/schemadiff` (introspect/diff via scratch execute-and-introspect), `pkg/planner` (classifier), `pkg/router` (backend assignment + availability policy), `pkg/plan` (versioned dry-run plan report), `pkg/lint` (offline typed findings), and `pkg/suggest` (advisory rewrites with typed caveats) exist (Phases 2.1–2.5) | (CO-7 holds at the parse boundary) |
 | `pkg/verdict` — structured outcome contract, rendering, exit codes | ❌ periphery | exists (Phase 1) | — |
-| `internal/cli` — CLI, flags, help, prompts | ❌ periphery | `migrate`, `status`, `diff`, `fmt`, and `lint` exist | — |
+| `internal/cli` — CLI, flags, help, prompts | ❌ periphery | `migrate`, `status`, `diff`, `fmt`, `lint`, and `suggest` exist | — |
 | status / progress / advisory rendering, metrics | ❌ periphery | planned | — |
 | orchestrator adapter | ❌ periphery | planned (Phase 11) | OC-* hold *at* the boundary |
 | `internal/testutil` | ❌ test-only | exists | — |

docs/low-level-design.md

@@ -288,7 +288,9 @@ request, not a permission.
 The [advisory behaviour](high-level-design.md#advisory-mode-suggest-the-safe-rewrite-dont-silently-run-the-risky-one)
 is a property of the **planner's classifier output**, not a separate code path. Every current
 `planner.Decision` carries the operation, route, typed reason, and safer SQL where applicable.
-The CLI renders that output in `diff` and `migrate --dry-run`.
+The CLI renders that output in `diff` and `migrate --dry-run`, and the offline `suggest`
+command (`pkg/suggest`) emits it as a standalone advisory report — original → recommended
+with typed reason and caveat metadata, never executing anything.
 
 ### What the classifier emits per operation
 

internal/cli/cli.go

@@ -1,6 +1,6 @@
 // Package cli defines the pg-sprite command tree (Kong): migrate and
 // status (the optimistic front door), diff and fmt (the declarative front
-// door), and lint (the offline checker).
+// door), and lint and suggest (the offline checker and advisor).
 package cli
 
 import (
@@ -23,6 +23,7 @@ type CLI struct {
 	Diff    DiffCmd    `cmd:"" help:"Diff a desired-state schema file against the live schema."`
 	Fmt     FmtCmd     `cmd:"" help:"Canonicalize a schema file."`
 	Lint    LintCmd    `cmd:"" help:"Lint DDL for unsafe patterns."`
+	Suggest SuggestCmd `cmd:"" help:"Recommend safer native forms for risky DDL."`
 	Status  StatusCmd  `cmd:"" help:"Report the status of a running migration."`
 }
 
@@ -121,6 +122,17 @@ type LintCmd struct {
 // Run implements the lint subcommand.
 func (c *LintCmd) Run() error { return c.runLint(os.Stdin, os.Stdout) }
 
+// SuggestCmd maps risky-as-written DDL to the safer native form the engine
+// would run instead, with typed caveats. It is offline and advisory — no
+// database flags, nothing executes, and it always exits zero.
+type SuggestCmd struct {
+	Path string `arg:"" optional:"" help:"DDL file to advise on; stdin when omitted." type:"existingfile"`
+	JSON bool   `help:"Emit the suggestions report as JSON."`
+}
+
+// Run implements the suggest subcommand.
+func (c *SuggestCmd) Run() error { return c.runSuggest(os.Stdin, os.Stdout) }
+
 // StatusCmd reports migration progress.
 type StatusCmd struct {
 	DBFlags `embed:""`

internal/cli/suggest.go

@@ -0,0 +1,73 @@
+package cli
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+
+	"github.com/block/pg-sprite/pkg/suggest"
+)
+
+// runSuggest maps a DDL script to its advisory rewrites: parse every
+// statement through the PostgreSQL grammar, classify it with zero live
+// facts, and report the safer native form for anything risky as written.
+// Offline — no database, nothing executes. A script with no rewrites
+// prints nothing and the command always exits zero: suggest advises, lint
+// gates.
+func (c *SuggestCmd) runSuggest(in io.Reader, out io.Writer) error {
+	var src []byte
+	var err error
+	if c.Path == "" {
+		if src, err = io.ReadAll(in); err != nil {
+			return fmt.Errorf("read DDL from stdin: %w", err)
+		}
+	} else if src, err = os.ReadFile(c.Path); err != nil {
+		return fmt.Errorf("read DDL file: %w", err)
+	}
+	report, err := suggest.Advise(string(src))
+	if err != nil {
+		return err
+	}
+	if c.JSON {
+		enc := json.NewEncoder(out)
+		enc.SetIndent("", "  ")
+		if err := enc.Encode(report); err != nil {
+			return fmt.Errorf("write suggest report: %w", err)
+		}
+		return nil
+	}
+	return writeSuggestText(out, report)
+}
+
+// writeSuggestText renders each suggestion as the original, the safer
+// sequence, and its caveats. A report with no suggestions prints nothing.
+func writeSuggestText(out io.Writer, report suggest.Report) error {
+	for _, s := range report.Suggestions {
+		if _, err := fmt.Fprintf(out, "statement %d: %s — %s\n", s.Statement, s.Operation, s.Reason); err != nil {
+			return fmt.Errorf("write suggest report: %w", err)
+		}
+		if _, err := fmt.Fprintf(out, "  safer form (not equivalent — see docs/postgres-online-ddl-reference.md):\n"); err != nil {
+			return fmt.Errorf("write suggest report: %w", err)
+		}
+		for _, sql := range s.Recommended {
+			if _, err := fmt.Fprintf(out, "    %s;\n", sql); err != nil {
+				return fmt.Errorf("write suggest report: %w", err)
+			}
+		}
+		if _, err := fmt.Fprintf(out, "  caveats: %s\n", joinCaveats(s.Caveats)); err != nil {
+			return fmt.Errorf("write suggest report: %w", err)
+		}
+	}
+	return nil
+}
+
+// joinCaveats renders the typed caveats as a comma-separated list.
+func joinCaveats(caveats []suggest.Caveat) string {
+	names := make([]string, len(caveats))
+	for i, c := range caveats {
+		names[i] = string(c)
+	}
+	return strings.Join(names, ", ")
+}

internal/cli/suggest_test.go

@@ -0,0 +1,78 @@
+package cli
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/block/pg-sprite/pkg/suggest"
+)
+
+func TestSuggestCleanScriptPrintsNothing(t *testing.T) {
+	var out strings.Builder
+	cmd := SuggestCmd{}
+	err := cmd.runSuggest(strings.NewReader("CREATE TABLE t (id int)"), &out)
+	require.NoError(t, err)
+	assert.Empty(t, out.String())
+}
+
+// Suggest is advisory: a script full of findings still exits zero — lint
+// owns the gate.
+func TestSuggestAlwaysExitsZeroOnValidScripts(t *testing.T) {
+	var out strings.Builder
+	cmd := SuggestCmd{JSON: true}
+	err := cmd.runSuggest(strings.NewReader(`
+		CREATE INDEX t_c_idx ON t (c);
+		ALTER TABLE t ADD CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =);
+	`), &out)
+	require.NoError(t, err)
+
+	var report suggest.Report
+	require.NoError(t, json.Unmarshal([]byte(out.String()), &report))
+	assert.Equal(t, suggest.FormatVersion, report.FormatVersion)
+	require.Len(t, report.Suggestions, 1, "the refused statement yields no advice")
+	assert.Equal(t, 1, report.Suggestions[0].Statement)
+}
+
+func TestSuggestReadsFromFile(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "change.sql")
+	require.NoError(t, os.WriteFile(path,
+		[]byte("ALTER TABLE t ALTER COLUMN c SET NOT NULL"), 0o600))
+
+	var out strings.Builder
+	cmd := SuggestCmd{Path: path, JSON: true}
+	require.NoError(t, cmd.runSuggest(strings.NewReader(""), &out))
+
+	var report suggest.Report
+	require.NoError(t, json.Unmarshal([]byte(out.String()), &report))
+	require.Len(t, report.Suggestions, 1)
+	assert.Equal(t,
+		[]suggest.Caveat{suggest.CaveatSeparateTransactions, suggest.CaveatValidationScan},
+		report.Suggestions[0].Caveats)
+}
+
+func TestSuggestParseFailureIsError(t *testing.T) {
+	var out strings.Builder
+	cmd := SuggestCmd{}
+	err := cmd.runSuggest(strings.NewReader("CREATE TABEL t (id int)"), &out)
+	require.Error(t, err)
+}
+
+// The text rendering is this renderer's own unit test: each suggestion
+// shows the safer sequence and its caveats.
+func TestSuggestTextRendering(t *testing.T) {
+	var out strings.Builder
+	cmd := SuggestCmd{}
+	err := cmd.runSuggest(strings.NewReader("CREATE INDEX t_c_idx ON t (c)"), &out)
+	require.NoError(t, err)
+	text := out.String()
+	assert.Contains(t, text, "statement 1:")
+	assert.Contains(t, text, "CONCURRENTLY")
+	assert.Contains(t, text, string(suggest.CaveatNonTransactional))
+	assert.Contains(t, text, string(suggest.CaveatInvalidIndexOnFailure))
+}

pkg/suggest/suggest.go

@@ -0,0 +1,161 @@
+// Package suggest is the advisory surface: it maps DDL that is risky as
+// written to the safer native form the engine would run instead, offline
+// and without executing anything. It reports only constructed rewrites —
+// refusals, table rewrites, and destructive drops are pkg/lint's job — and
+// every recommendation carries typed caveats, because a safer form is not
+// a semantic equivalent: it reaches the same end state with different
+// locking, transactionality, and failure modes.
+package suggest
+
+import (
+	"fmt"
+
+	"github.com/block/pg-sprite/pkg/planner"
+	"github.com/block/pg-sprite/pkg/statement"
+)
+
+// FormatVersion identifies the report contract. A consumer must reject a
+// report whose version it does not understand instead of guessing at the
+// field semantics.
+const FormatVersion = 1
+
+// Caveat is a typed condition attached to a recommendation; automation
+// branches on it, never on prose.
+type Caveat string
+
+// The caveats a recommendation can carry.
+const (
+	// CaveatNonTransactional: the recommended sequence contains a
+	// CONCURRENTLY statement, which cannot run inside a transaction
+	// block.
+	CaveatNonTransactional Caveat = "non-transactional"
+	// CaveatSeparateTransactions: the steps must commit separately — the
+	// weaker locks the sequence exists for are held to commit, so one
+	// enclosing transaction reproduces the blocking the rewrite avoids.
+	CaveatSeparateTransactions Caveat = "separate-transactions"
+	// CaveatInvalidIndexOnFailure: a failed or cancelled concurrent build
+	// leaves an INVALID index that must be detected (pg_index.indisvalid)
+	// and dropped or rebuilt; the engine's executor owns that check when
+	// it runs the sequence.
+	CaveatInvalidIndexOnFailure Caveat = "invalid-index-on-failure"
+	// CaveatDetachFinalizeOnFailure: an interrupted concurrent detach
+	// leaves the partition half-detached; it must be finished with
+	// DETACH PARTITION FINALIZE.
+	CaveatDetachFinalizeOnFailure Caveat = "detach-finalize-on-failure"
+	// CaveatValidationScan: the VALIDATE step still scans every row — the
+	// rewrite trades the lock strength, not the scan.
+	CaveatValidationScan Caveat = "validation-scan"
+)
+
+// Suggestion is one advisory rewrite: the statement as written, the safer
+// native sequence, and the typed metadata explaining the trade.
+type Suggestion struct {
+	// Statement is the 1-based index of the statement in the script.
+	Statement int `json:"statement"`
+	// Original is the canonical text of the statement as submitted.
+	Original string `json:"original"`
+	// Operation is the operator-facing label of the risky operation
+	// (display only).
+	Operation string `json:"operation"`
+	// Reason is the classifier's typed cause for preferring the rewrite.
+	Reason planner.Reason `json:"reason"`
+	// Recommended is the ordered safer SQL to run instead.
+	Recommended []string `json:"recommended"`
+	// Caveats are the typed conditions under which the recommendation
+	// differs from the original; never empty — a rewrite with no trade
+	// would be the same statement.
+	Caveats []Caveat `json:"caveats"`
+}
+
+// Report is the advisory result for one script.
+type Report struct {
+	// FormatVersion is the report contract version; always FormatVersion.
+	FormatVersion int `json:"format_version"`
+	// Suggestions are the rewrites in statement order; empty means every
+	// statement is already in its safest known form or is outside the
+	// advisory surface (refusals and rewrites are lint findings).
+	Suggestions []Suggestion `json:"suggestions"`
+}
+
+// Advise maps a DDL script to its advisory rewrites: every statement is
+// parsed with the PostgreSQL grammar and classified with zero live facts,
+// and each risky-as-written operation with a constructible safer form
+// yields a Suggestion. Nothing is executed and no database is touched. A
+// parse failure is an error.
+func Advise(sql string) (Report, error) {
+	stmts, err := statement.Split(sql)
+	if err != nil {
+		return Report{}, err
+	}
+	report := Report{FormatVersion: FormatVersion, Suggestions: []Suggestion{}}
+	for i, stmt := range stmts {
+		suggestions, err := adviseStatement(i+1, stmt)
+		if err != nil {
+			return Report{}, fmt.Errorf("statement %d: %w", i+1, err)
+		}
+		report.Suggestions = append(report.Suggestions, suggestions...)
+	}
+	return report, nil
+}
+
+// adviseStatement produces the suggestions for one statement: one per
+// safer-idiom decision whose rewrite the planner could construct. The
+// operation list and decision list are index-aligned by the planner's
+// contract (one decision per operation, in order); a mismatch is a
+// contract violation and fails closed.
+func adviseStatement(index int, sql string) ([]Suggestion, error) {
+	plan, err := planner.Classify(sql, planner.Facts{})
+	if err != nil {
+		return nil, err
+	}
+	ops, err := statement.ParseOps(sql)
+	if err != nil {
+		return nil, err
+	}
+	if len(ops) != len(plan.Decisions) {
+		return nil, fmt.Errorf("planner produced %d decisions for %d operations", len(plan.Decisions), len(ops))
+	}
+	var suggestions []Suggestion
+	for i, d := range plan.Decisions {
+		if d.Reason != planner.ReasonSaferIdiom || len(d.SaferSQL) == 0 {
+			continue
+		}
+		caveats, err := rewriteCaveats(ops[i])
+		if err != nil {
+			return nil, err
+		}
+		suggestions = append(suggestions, Suggestion{
+			Statement:   index,
+			Original:    sql,
+			Operation:   d.Operation,
+			Reason:      d.Reason,
+			Recommended: d.SaferSQL,
+			Caveats:     caveats,
+		})
+	}
+	return suggestions, nil
+}
+
+// rewriteCaveats maps an operation to the typed caveats of its safer
+// rewrite. An operation with a rewrite this table does not know is a
+// contract violation — when the planner learns a new rewrite, its caveats
+// must be recorded here before the advice ships — so it fails closed
+// rather than emitting caveat-less advice.
+func rewriteCaveats(op statement.Op) ([]Caveat, error) {
+	switch op.Kind {
+	case statement.OpCreateIndex, statement.OpDropIndex, statement.OpReindex:
+		return []Caveat{CaveatNonTransactional, CaveatInvalidIndexOnFailure}, nil
+	case statement.OpDetachPartition:
+		return []Caveat{CaveatNonTransactional, CaveatDetachFinalizeOnFailure}, nil
+	case statement.OpSetNotNull:
+		return []Caveat{CaveatSeparateTransactions, CaveatValidationScan}, nil
+	case statement.OpAddConstraint:
+		switch op.Constraint {
+		case statement.ConstraintPrimaryKey, statement.ConstraintUnique:
+			return []Caveat{CaveatNonTransactional, CaveatInvalidIndexOnFailure}, nil
+		case statement.ConstraintCheck, statement.ConstraintForeignKey:
+			return []Caveat{CaveatSeparateTransactions, CaveatValidationScan}, nil
+		}
+	}
+	return nil, fmt.Errorf("no caveat mapping for rewritten operation %q", op.Describe())
+}