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>

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>

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>

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>

arises

Previous wording said "the catalog returns 0", which implied MySQL
itself reports a literal 0 for indexes without a prefix length. It
doesn't — `information_schema.STATISTICS.SUB_PART` is NULL in that
case. The catalog loader scans it into a `*int`; when nil it leaves
`model.IndexPart.Length` at the struct zero-value (0). vitess's
`IndexColumn.Length` is the symmetric `*int` on the parser side.

Reword the bullet so it describes both sides as `*int` carrying nil
for "no prefix", and call out that the equal-at-0 comparison is the
result of the struct default — not a quirk of MySQL's response.

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.

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>