Seven inline comments, all valid. Fixes:

parser/directive.go
  - Whitespace tolerance: classifyInlineLine() now goes through
    strings.Fields/tokenize() so tabs and runs of multiple spaces
    between KEY/INDEX/UNIQUE/etc. and the name don't defeat the
    kind classifier. (Copilot #1, two comments on the same issue.)
  - Kind-aware extraction: ExtractInlineRenames() returns
    *InlineRenames with separate Columns / Indexes maps plus an
    Unsupported list, so a column and an index that share a name
    (which happens when a KEY auto-names after its first column)
    no longer compete for the same map key. (Copilot #2.)
  - Directives that resolve to constraints / FKs / PRIMARY KEY /
    anonymous FOREIGN KEY / unrecognised line shapes are now
    recorded in Unsupported instead of silently dropped — the
    parser turns them into errors. (Copilot #3.)
  - renameDirectivePattern no longer accepts `.` in the old name;
    the surrounding comment now matches reality (qualified names
    were never plumbed through and are intentionally rejected).
    (Copilot #6.)
  - ExtractInlineRenames skips the leading-comment block (those
    are statement-level, owned by ExtractStmtRenameFrom) so the
    statement-level directive doesn't get re-attached to the
    `CREATE TABLE …` opener line.

parser/parser.go
  - Uses InlineRenames.Columns / .Indexes to attach RenameFrom by
    kind, not by flat lookup. Errors out on Unsupported entries
    (constraint / FK / dangling directive) so a typo or
    mis-positioned directive fails the parse instead of silently
    degrading into a destructive DROP+CREATE.

diff/rename.go
  - Removes the misleading trailing comment in applyColumnRenames
    (the "propagate to indexes" work happens in the caller, not
    here). (Copilot #4.)
  - Adds rewriteFKColumnRefs(), the FK counterpart to
    rewriteIndexColumnRefs: rewrites FK Columns entries from
    old → new after a column rename so fkEqual stays quiet. Without
    this, a plain column rename emitted DROP FOREIGN KEY +
    ADD CONSTRAINT for any FK on the renamed column. (Copilot #5.)

diff/tables.go
  - Calls rewriteFKColumnRefs after applyColumnRenames.

Tests
  - parser/directive_test.go: rewritten for the kind-aware API,
    plus new cases for whitespace tolerance (tab, multi-space),
    column/index name collision, qualified-name rejection,
    constraint-target = unsupported, dangling-directive = error,
    full ParseSQL error wrapping.
  - diff/rename_test.go: new TestDiffRenameColumnAlsoRewritesFKColumns
    covers the FK-rewrite path end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Four inline comments, all valid; one is a real bug.

diff/rename.go (BUG fix)
  - applyTableRenames now also rewrites (RefDB, RefTable) on every
    other table's FKs that pointed at the old name. Without this, a
    pure table rename (shop.users → shop.members) made every FK on
    referencing tables diff as DROP FOREIGN KEY + ADD CONSTRAINT,
    and would fail to apply under restrictive --allow-drop=foreign_key
    settings even though only the target name changed.
    (Copilot #4.)

parser/directive.go
  - ValidateDirectives now also checks the *syntax* of recognised
    directives, not just the name. Malformed renamed-from lines
    (qualified names like `db.tbl`, missing argument, trailing junk)
    fail at validation time with a clear "malformed" message instead
    of being silently dropped by the extractor and degrading to
    DROP+CREATE. (Copilot #1.)
  - renameDirectivePattern accepts any backtick-quoted blob as the
    old name, mirroring model.Ident's behaviour for reserved words /
    hyphens / etc. Bareword names still take the existing identifier
    grammar. (Copilot #2.)
  - classifyInlineLine detects the unnamed `KEY (col)` /
    `INDEX (col)` form (tokens[1] starts with "(") and returns
    inlineKindUnknown so the directive is reported as unsupported,
    rather than later surfacing as a confusing "target index '(col)'
    not found" error. (Copilot #3.)

Tests
  - parser/directive_test.go: new cases for qualified-name rejection
    at validate time, missing-arg, trailing-junk, backticked reserved
    word in the directive arg, and unnamed `KEY (col)` form being
    flagged as unsupported. The previously existing "qualified name
    silently rejected" test is reframed to document that it's the
    extractor's behaviour while ValidateDirectives now errors loudly.
  - diff/rename_test.go: TestDiffRenameTableAlsoRewritesCrossTableFKRefs
    covers the cross-table FK rewrite end-to-end (renames
    shop.users → shop.members and asserts shop.posts' FK to it stays
    quiet — no DROP FOREIGN KEY, no ADD CONSTRAINT).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Five inline + one suppressed comment, all valid.

parser/directive.go
  - ExtractStmtRenameFrom now returns (string, error) and errors when
    more than one renamed-from directive appears in the leading
    comment block — multiple sources is ambiguous and almost always a
    typo, so failing loudly beats silently letting the last one win.
    (Copilot #5.)
  - ExtractInlineRenames detects two `-- myschema:renamed-from` lines
    stacked with no SQL line between them; the first directive (which
    never attached to anything) is now appended to Unsupported instead
    of being silently overwritten. (Copilot #3.)
  - classifyInlineLine handles column lines whose name is backtick-
    quoted with embedded whitespace (`weird name VARCHAR(64)`) via a
    new leadingBacktickedIdent helper. strings.Fields can't tokenize
    these correctly because the name itself contains spaces; the
    helper does a backtick-aware first-identifier parse. KEY / INDEX
    / CONSTRAINT keywords are MySQL reserved tokens and never quoted,
    so a leading backtick can only be a column name. (Copilot #4.)
  - Doubled backticks inside a backticked old-name (MySQL's escape
    for an embedded backtick) remain rejected by renameDirectivePattern;
    the validator's "malformed directive" error is the right surface.
    Documented as out of scope in the leadingBacktickedIdent comment.
    (Copilot #1, #2.)

parser/parser.go
  - rejectMisplacedRenameDirectives() errors when a renamed-from
    directive is extracted but the surrounding statement isn't
    CREATE TABLE (currently the only kind that consumes directives).
    Wired into the AlterTable, CreateView, and default switch arms
    so a directive on a CREATE VIEW etc. fails the parse instead of
    being silently dropped on the floor and degrading the next plan
    into a destructive DROP+CREATE. (Suppressed Copilot comment on
    parser.go:96.)
  - ExtractStmtRenameFrom call now propagates the new error.

Tests
  - parser/directive_test.go: existing call sites updated for the
    new (string, error) signature; new cases for stacked directives,
    multiple stmt directives error, backticked column with embedded
    space, and renamed-from on CREATE VIEW being rejected by ParseSQL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Seven inline comments, all valid; two are real bugs.

parser/directive.go
  - End-of-loop guard in ExtractInlineRenames: a `pending` directive
    whose target line never arrived (statement body ran out) is now
    appended to Unsupported instead of being silently dropped.
    (Copilot #1.)
  - leadingBacktickedIdent comment rewritten: previous wording had
    curly quotes ("…" instead of `…`) and didn't explain MySQL's
    backtick-doubling escape. New comment is precise: doubled
    backticks remain unsupported; the regex correctly rejects them
    so the validator surfaces "malformed directive". (Copilot #2, #3.)

diff/rename.go
  - applyTableRenames / applyColumnRenames / applyIndexRenames now
    short-circuit when RenameFrom equals the desired name. This
    avoids generating `ALTER TABLE x RENAME TO x` (rejected on some
    MySQL versions, no-op on others) for what is almost always a
    user typo. (Copilot #5, #6, #7.)
  - rewriteConstraintColumnRefs: PRIMARY KEY column lists in
    current.Constraints are rewritten old → new alongside the index
    and FK rewrites, so a renamed-PK column doesn't surface as
    DROP+ADD PRIMARY KEY in diffConstraints. CHECK constraints are
    deliberately skipped — their Definition is a free-form
    expression and rewriting it requires a full SQL parser.
    (Copilot #4.)

diff/tables.go
  - Calls rewriteConstraintColumnRefs after applyColumnRenames,
    alongside the existing index / FK rewrites.

Tests
  - parser/directive_test.go: TestExtractInlineRenamesTrailingPendingUnsupported
    covers the end-of-loop pending path.
  - diff/rename_test.go: TestDiffRenameSelfRenameIsNoOp covers the
    table / column / index self-rename guards in one shot.
    TestDiffRenameColumnAlsoRewritesPKConstraint covers the new PK
    rewrite end-to-end (renames `users.old_id` → `users.id` and
    asserts no DROP/ADD PRIMARY KEY appears).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Four inline comments, all valid; one is a real bug.

diff/rename.go + diff/tables.go (BUG fix)
  - rewriteCrossTableFKRefCols(): for every desired table whose
    columns carry RenameFrom, walk all *other* current tables and
    rewrite RefCols on FKs that reference (db, table) when the
    referenced column matches one of the renames. Without this, a
    pure column rename on the parent side (`users.id` → `users.user_id`)
    diff'd every referencing FK as DROP+ADD even though MySQL
    updates the parent-side reference automatically. (Copilot #1.)
  - DiffTables runs the cross-table pass after table renames and
    before the modified-tables loop, so per-table diffTable() sees
    a consistent view.

diff/tables.go (doc)
  - DiffTables doc-comment now warns that rename handling mutates
    `current` in place (re-keys after table rename, updates Name /
    Database / FK Columns / RefCols / index parts / PK constraint
    columns). Production callers in diff_all.go build a fresh
    `current` per invocation, so this is fine; tests that share
    fixtures across subtests should clone first. (Copilot #2.)

parser/directive.go
  - ExtractInlineRenames now skips `# ...` line comments and
    single-line `/* ... */` block comments between a directive and
    its target. Multi-line `/* ... */` is still not unwound — a rare
    case in hand-written CREATE TABLE bodies — and is documented in
    the func comment. Without this, a stray `# foo` between the
    directive and the next column line would have been treated as
    SQL and the directive mis-attached or surfaced as
    "target not found". (Copilot #3.)
  - leadingBacktickedIdent comment rewritten *for real* this time
    (the previous attempt left U+201C "left double quotation mark"
    in place where ASCII backticks were intended). The comment now
    describes MySQL's escape rule in plain English without any
    smart-quote characters. (Copilot #4.)

Tests
  - parser/directive_test.go: TestExtractInlineRenamesSkipsBlockAndHashComments
    covers the new comment-skipping logic.
  - diff/rename_test.go: TestDiffRenameColumnAlsoRewritesCrossTableFKRefCols
    renames `users.id` → `users.user_id` while `posts.fk_user`
    references it, and asserts the FK on posts stays quiet (no
    DROP FOREIGN KEY, no ADD CONSTRAINT) — the cross-table RefCols
    rewrite is exercised end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Four inline comments. Two typo nits, two flow consistency fixes.

Typos
  - parser/directive.go:415 — comment for stripUntilAfterBacktickedName
    contained a curly close-quote (\`name\“) instead of a backtick.
    Replaced. (Copilot #1.)
  - diff/rename_test.go:390 — comment said "RenameFrm"; should be
    "RenameFrom". Fixed. (Copilot #2.)

parser/directive.go (flow consistency)
  - ValidateDirectives now goes through the same
    reduceLeadingBlocks + multi-line block state path the extractors
    use, so a malformed directive after `/* header */` or after
    `*/` on the same multi-line block-close line still errors at
    validation time. Without this, the validator and extractor
    disagreed on what counts as a "directive line", letting bad
    directives slip past validation and be silently ignored.
    (Copilot #3.)
  - ExtractStmtRenameFrom and ExtractInlineRenames re-process the
    suffix after a multi-line `*/` close on the same line, so
    `*/ -- myschema:renamed-from old` still attaches. Previously
    the code cleared inBlock and `continue`'d, dropping the
    directive. (Copilot #4.)

Tests
  - TestValidateDirectivesAfterLeadingBlockComment
  - TestValidateDirectivesAfterMultiLineBlockClose
  - TestExtractStmtRenameFromMultiLineBlockCloseSameLineAsDirective
  - TestExtractInlineRenamesMultiLineBlockCloseSameLineAsDirective

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two remaining inline comments (the other two were already addressed
in earlier commits of this branch):

test/scenario/helper.sh
  - The default `_base_dsn` no longer hardcodes 127.0.0.1:3306; it's
    now built from MYSQL_USER / MYSQL_HOST / MYSQL_PORT, so the
    mysql CLI used by setup_db and the myschema driver target the
    same instance even when only MYSQL_PORT is overridden.
    Reordered the var-init block so MYSQL_HOST / MYSQL_PORT /
    MYSQL_USER are resolved before _base_dsn references them.
    (Copilot #2.)

Makefile
  - test-mysql9 / clean-schema-mysql9 stop hardcoding the full DSN
    and just delegate to the existing `test` / `clean-schema`
    targets with MYSQL_PORT=3307. The MYSCHEMA_TEST_DSN template at
    the top of the Makefile is recursively expanded, so the new port
    flows through to the DSN and any MYSQL_HOST / MYSQL_USER
    customisation carries with it. (Copilot #3.)

Verified locally: `make test-mysql9` and `make test-scenario
MYSQL_PORT=3307` both pass against the running 9.x compose service.

Copilot #1 (Makefile `:=` overriding env) was fixed in 0320bfc.
Copilot #4 (compose.yaml comment about profile) was updated when
the profile itself was removed in 1f51540.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Five inline comments, all valid.

helper.sh
  - assert_contains / assert_not_contains now capture stderr too
    (`2>&1`) so a command that prints the relevant text to stderr
    doesn't cause a false negative. (Copilot #1, #2.)
  - Both helpers explicitly validate the invocation: missing `--`
    separator, no command before `--`, or no substring after `--`
    each fail with a clear message instead of letting bash run a
    malformed command and produce an opaque "command not found"
    or hang on stdin. (Copilot #1, #2.)
  - Sanity-checked all four validation paths (`missing sep`,
    `missing cmd`, `missing substring`, `stderr capture`) by
    sourcing helper.sh in a one-shot bash session.

dump_roundtrip.test.sh
  - The dump-failure and apply-failure branches now `summary; exit 1`
    after `fail`, instead of falling through to the next step. The
    cascading errors that followed obscured the original failure;
    failing fast keeps the scenario log focused on the root cause.
    (Copilot #3, #4.)

evolution.test.sh
  - Header comment said "ADD COLUMN with DEFAULT" but the fixture
    drops the DEFAULT (workaround for the catalog default-
    normalisation drift documented in TODO.md). Comment now matches
    reality and points at the TODO entry. (Copilot #5.)

`make test-scenario` (mysql 8.0) — all 22 steps still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot review #4 style nit: the long line with the parenthetical
aside broke the rest of TODO.md's wrapping convention, and inline
`SELECT ` (with trailing space) is brittle in renderers that
collapse trailing whitespace. Rewrapped at the standard width and
turned the parenthetical into its own sentence; the inline code is
now `SELECT` followed by the prose "with an empty expression".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Five Copilot inline comments on PR #31, all valid:

helper.sh
  - assert_contains / assert_not_contains validate $# >= 1 before
    referencing $1 / shifting. Under `set -u` (which the scenario
    runner enables), a no-arg call would otherwise abort with
    "unbound variable" instead of falling into the helper's own
    fail() path. (Copilot #1, #2.)
  - Replace `echo "$out" | grep ...` with
    `printf '%s\n' "$out" | grep -qF -- ...`. echo can mangle
    output that starts with `-n` / contains escape sequences;
    printf is faithful, and the `--` after `-qF` keeps grep from
    treating a leading `-` in the substring as an option.
    (Copilot #3, #4.)

test/scenario/dump_roundtrip.test.sh
  - `mktemp -d` without a template is GNU-specific; macOS / BSD
    requires one. Pass an explicit template under `$TMPDIR` so the
    scenario runs portably. (Copilot #5.)

`make test-scenario` (mysql 8.0) — all 24 steps still pass.
Sanity-checked the `set -u` path by sourcing helper.sh in a fresh
bash session and calling `assert_contains` with no args; it now
returns via fail() instead of aborting.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Four Copilot inline comments on PR #33:

catalog/tables.go (doc) + TODO.md
  - Doc and TODO still listed VARBINARY alongside BINARY as "needs
    its own normalisation". The previous round actually moved
    VARBINARY into the supported set (it surfaces its DEFAULT '' as
    the bare empty string, so the existing path round-trips it
    cleanly). Update the function-doc and TODO to spell out that
    only fixed-width BINARY remains broken. (Copilot #1, #2.)

testdata/apply/empty_string_default_no_drift.yml
  - Add a VARBINARY(8) column to the no-drift fixture so the new
    branch is exercised through plan + apply + drift-check, not
    just the catalog unit test. (Copilot #3.)

catalog/tables.go + catalog/catalog_test.go (typography)
  - Bulk replace stray U+201D ("right double quotation mark") with
    ASCII `''` across both files. The previous round's Edit tool
    ate the quote characters; verified the bytes with a follow-up
    grep. (Copilot #4.)

`make test` (8.0) — pass; `make test-scenario` (8.0) — pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

case

Previous wording said the empty-string default was the *only*
type-aware path in `normalizeColumnDefault`. That was true at one
point but PR #34 added a second type-aware branch: a fixed-width
`BINARY(N)` column with `DEFAULT ''` surfaces from MySQL as the
literal two-character string `"0x"` (independent of N), and the
catalog rewrites that sentinel back to `''` when the type-name
starts with `binary`.

Restructure the bullet into a short three-case list (non-empty
type-agnostic / empty-string type-aware / BINARY(N) "0x" type-aware)
so the docs match the code.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

"reset to default" semantics

Two more convergence holes the previous pass left open. Both stem
from over-reaching the simplifications I made earlier.

#1 parser table-level COLLATE-only

Last pass added effective-charset fallback for column-level COLLATE-
only, but the *table-level* equivalent was still left bare:

    CREATE TABLE t (...) COLLATE=utf8mb4_0900_ai_ci;

(no DEFAULT CHARSET) → parser had t.Charset=nil so
CollapseDefaultCollation couldn't fire, while catalog (which always
knows the effective charset via information_schema.COLLATIONS) did
collapse — endless `ALTER TABLE … COLLATE=…` loops.

Add `model.CharsetOfCollation(coll)` (splits on the first `_`, with
the `binary` self-named special case) and a parser-side
`effectiveCharsetForCollation` helper that walks the chain
(declared charset → table default → derived from collation name)
before calling CollapseDefaultCollation. Used for both table-level
and column-level normalisation.

#2 diff don't-care semantics was too loose

Last pass treated `desired.Collation == nil` as "don't care", which
silently swallowed a real intent: dropping COLLATE from desired SQL
while keeping DEFAULT CHARSET means "reset to the charset's default
collation". With the catch-all don't-care, that case never emitted
an ALTER even when the catalog carried an explicit non-default
collation.

Refine the comparison:
- desired.Collation != nil → strict ptrEq (unchanged).
- desired.Collation == nil && desired.Charset != nil → catalog must
  be at default (current.Collation == nil after collapse); otherwise
  emit `ALTER TABLE … DEFAULT CHARSET=…` so MySQL resets.
- desired.Charset == nil treated as "no opinion on charset".
- Both nil → handled by the existing early return.

Tests
- model: TestCharsetOfCollation pins prefix split, the `binary`
  special case, mixed-case input, and the no-underscore fallback.
- parser: TestParseTableOptionsCollateOnly has two sub-tests — the
  default collation collapses to nil, a non-default one survives.
- diff: testdata/plan/charset_drop_explicit_collation.yml — catalog
  has explicit COLLATE=utf8mb4_unicode_ci, desired drops it but
  keeps DEFAULT CHARSET=utf8mb4 → plan emits a single
  `ALTER TABLE … DEFAULT CHARSET=utf8mb4` (no COLLATE).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#1 Pin the RANGE COLUMNS REORGANIZE end-to-end. The plan-only
   fixture (partition_value_change_reorganize_range_columns.yml)
   covered the generator side, but nothing confirmed MySQL accepts
   `REORGANIZE PARTITION ... INTO (...)` with tuple boundaries or
   that the rewritten boundaries round-trip cleanly. Add the
   matching apply / verify_no_drift fixture.

#2 The case-only partition-name no-op already has a regression test
   (`partition_name_case_only_diff_no_op.yml`, commit 4c6998f).

#3 Pin the per-partition *option*-only diff (here: COMMENT-only)
   end-to-end. partitionDefEqual delegates to
   `parser.FormatPartitionDefinition`, which preserves COMMENT /
   MAX_ROWS / TABLESPACE / etc., so any byte-different definition
   already routed through the REORGANIZE branch — but no fixture
   pinned that MySQL accepts the generated statement or that the
   rewritten options round-trip. New apply fixture
   `partition_per_partition_option_change_reorganize.yml` covers
   both. LIST avoids the RANGE upper-bound cascade so the
   REORGANIZE only touches the one slot whose option actually
   changed.

#4 / #5 / #6 Widen the Coverage / Not-yet-implemented sections of
   AGENTS.md (in scope + v1 cuts) and the TODO.md "Already shipped"
   block from "pure value-change (same names, only `VALUES`
   differs)" to "per-partition definition rewrite ... when both
   sides have the same partition names in the same order — covers
   `VALUES …` boundary tweaks plus COMMENT / MAX_ROWS /
   TABLESPACE / other per-partition option changes that round-trip
   through vitess's PartitionDefinition formatter". Brings the
   docs in line with what diffPartitions actually does (and with
   what CAVEATS.md "Per-partition definition change" already
   describes).

#7 Fix the misleading comment in partitionNameListEqual.
   `parser.NormalizePartitionOption` only lower-cases function /
   column-reference identifiers inside the partition expression —
   it does NOT lower-case partition names. The case-insensitive
   `EqualFold` here is needed precisely because the round-trip
   preserves whatever case the user wrote on the partition name.

- TODO: rewrite the CHECK NOT ENFORCED entry to point at the real
  gap (catalog/tables.go hard-codes Enforced=true) — model and diff
  layers already plumb the flag.
- TODO: rewrite the table-level COMMENT entry to point at the diff
  layer (Engine/Comment intentionally not diffed) — model and
  catalog already plumb the field.
- testdata/apply/add_json_column.yml: COLUMN_TYPE, not DATA_TYPE,
  matches what catalog/tables.go loadColumns actually selects.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Round 1 review on PR #75 caught eight issues; this commit addresses
all of them.

1. (review #1) loadPreSQL treated any non-empty PreSQL string as
   "set", including whitespace-only values. Whitespace-only env
   var (`MYSCHEMA_PRE_SQL=`) would trigger the mutually-exclusive
   error against a legitimate --pre-sql-file, and skip the no-op
   short-circuit. Fix: TrimSpace both fields before deciding "set".

2. (review #2) loadPreSQL read --pre-sql-file with os.ReadFile,
   which doesn't support the repo's existing `-` for stdin
   convention used by parser.ReadSQLFile (the desired-SQL file
   args already accept it). Fix: route through parser.ReadSQLFile
   so `--pre-sql-file=-` works.

3. (review #3 + #4) runPreSQL was invoked AFTER connect(), so
   flag-validation errors (both flags set, missing file) could be
   masked by a downstream connection failure, and the client
   opened a DB connection it then threw away. Split into:
   - loadPreSQL: validate / read, no DB contact (called BEFORE connect)
   - execPreSQL: run on conn (called AFTER connect)
   apply.go and plan.go updated accordingly.

4. CLI-level mutual exclusion: tag both PreSQL and PreSQLFile with
   kong's `xor:"pre-sql"` so kong rejects the both-set case at
   parse time before our code sees it. The runtime check in
   loadPreSQL stays in place for programmatic API callers
   (Apply / Plan invoked from Go without going through kong).

5. (review #5 / #6 / #7 / #8) Test reshuffle. The original
   suite over-relied on success-only smoke tests that would have
   silently passed even if pre-SQL were skipped:
   - TestApply_PreSQLString and TestApply_PreSQLMultiStatement
     only checked NoError.
   - TestApply_PreSQLAppliesToSession claimed to be the
     "strongest behavioural pin" but probed nothing — the
     connection it would have probed is closed by Apply's defer.
   - TestPlan_PreSQLString same issue.
   Fix: load-bearing pins now feed INVALID pre-SQL and assert
   the apply / plan aborts with a wrapped "pre-sql" error
   containing the exact failing piece. The multi-statement
   split test now uses a payload with an invalid SECOND piece
   and asserts the error references that piece exactly (proves
   the split happened — without splitting, the driver would
   reject the whole concatenated string with a different error).

6. YAML migration: the new harness field `pre_sql` lets
   inline-payload tests live as testdata/apply/pre_sql_*.yml and
   testdata/plan/pre_sql_*.yml fixtures (3 + 1 = 4 new fixtures).
   File-related and programmatic-API tests stay in Go because
   the YAML harness has no `pre_sql_file` field (would need
   fixture-relative path handling, out of scope).

Coverage is unchanged at the package level; loadPreSQL stays at
100% and execPreSQL is exercised by all the real-execution paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- --split help text now mentions the rejected character set ("Object
  names containing path separators or ':' are rejected at write
  time"). MySQL backtick-quotes accept identifiers like `foo:bar`
  that splitPath then refuses; the help text steers users with such
  schemas to concat dump (or to rename the table) instead of
  failing without a hint.
- All split-mode tests now build paths with filepath.Join — both
  the per-package dump_test.go and the cmd/command/dump_test.go.
  The string-concat form `dir+"/x.sql"` is non-portable on Windows
  and inconsistent with the rest of the repo. 17 sites converted
  in dump_test.go, 1 in cmd/command/dump_test.go.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Probe fixture revealed a real bug: MySQL rejects

  ALTER TABLE users RENAME COLUMN old_name TO name, MODIFY COLUMN name varchar(128) NOT NULL;

with `Error 1054 (42S22): Unknown column 'name' in 'users'`. Within
one ALTER, MySQL resolves spec-target identifiers (`MODIFY COLUMN
<name>`, `DROP INDEX <name>`, …) against the *original* table state,
so any spec referring to the renamed object by its new name fails.
The diff layer emits column rename + a follow-on MODIFY (or index
rename + follow-on DROP) into Stmts as two separate ALTERs, which
--bulk-alter was happily folding into the broken combined form.

Fix: splitCombinableAlter rejects any spec starting with
`RENAME COLUMN` or `RENAME INDEX`, so renames keep their own ALTER
even under --bulk-alter. Whole-table `RENAME TO` lives in the
RenameStmts bucket upstream and never reaches the combiner, so no
guard needed for that shape.

Test changes:
- New unit tests pin the RENAME COLUMN and RENAME INDEX run-separator
  behaviour (both 100% covered).
- testdata/apply/bulk_alter_rename_then_modify_column.yml is the
  end-to-end regression — would fail with 1054 without the guard.
- testdata/apply/bulk_alter_renamed_column_with_followup.yml updated
  to expect the post-fix two-ALTER shape (was previously asserting
  the unsafe combined form, which only worked because the follow-on
  ADD COLUMN happened to use AFTER instead of MODIFY).
- testdata/apply/bulk_alter_charset_with_column_change.yml: new
  fixture for Copilot review #4, pinning DEFAULT CHARSET combined
  with ADD COLUMN. The CHARSET / column two-apply convergence
  (CAVEATS.md) is unchanged by --bulk-alter — verify_no_drift: false
  matches change_table_charset.yml's baseline.

CAVEATS.md / AGENTS.md updated with the rename trap and rationale.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Probe fixture revealed a real bug: MySQL rejects

  ALTER TABLE users RENAME COLUMN old_name TO name, MODIFY COLUMN name varchar(128) NOT NULL;

with `Error 1054 (42S22): Unknown column 'name' in 'users'`. Within
one ALTER, MySQL resolves spec-target identifiers (`MODIFY COLUMN
<name>`, `DROP INDEX <name>`, …) against the *original* table state,
so any spec referring to the renamed object by its new name fails.
The diff layer emits column rename + a follow-on MODIFY (or index
rename + follow-on DROP) into Stmts as two separate ALTERs, which
--bulk-alter was happily folding into the broken combined form.

Fix: splitCombinableAlter rejects any spec starting with
`RENAME COLUMN` or `RENAME INDEX`, so renames keep their own ALTER
even under --bulk-alter. Whole-table `RENAME TO` lives in the
RenameStmts bucket upstream and never reaches the combiner, so no
guard needed for that shape.

Test changes:
- New unit tests pin the RENAME COLUMN and RENAME INDEX run-separator
  behaviour (both 100% covered).
- testdata/apply/bulk_alter_rename_then_modify_column.yml is the
  end-to-end regression — would fail with 1054 without the guard.
- testdata/apply/bulk_alter_renamed_column_with_followup.yml updated
  to expect the post-fix two-ALTER shape (was previously asserting
  the unsafe combined form, which only worked because the follow-on
  ADD COLUMN happened to use AFTER instead of MODIFY).
- testdata/apply/bulk_alter_charset_with_column_change.yml: new
  fixture for Copilot review #4, pinning DEFAULT CHARSET combined
  with ADD COLUMN. The CHARSET / column two-apply convergence
  (CAVEATS.md) is unchanged by --bulk-alter — verify_no_drift: false
  matches change_table_charset.yml's baseline.

CAVEATS.md / AGENTS.md updated with the rename trap and rationale.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address Copilot review on PR #79 (3 of 4 items): three sites in
README and getting-started described `--include`/`--exclude` as
table-only filters, but `diff_all.go`'s filterViews calls the same
FilterOptions.MatchName helper, so the flags scope views too.
Updated all three callsites to say "table and view names".

Skipping the source-build PATH note (#4) — install instructions
are slated to change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>