feat(cli): Add review subcommand with eight migration risk rules#91
Merged
Conversation
Foreign keys whose only modification was a referential action change were previously reported as Unchanged because fks_differ ignored on_delete and on_update. Compare those fields so cascade behavior shifts surface in diff output, which Phase 1 review rules (risk/add-cascade-delete) will rely on.
Introduces the relune-core::review module that compares before/after schemas and surfaces migration-time risks distinct from the existing lint engine. Implements eight rules covering dropped references, narrowing type changes, NOT NULL on existing data, dropped PK/UNIQUE, new UNIQUE on existing data, ON DELETE CASCADE additions, and new foreign keys without supporting indexes. ReviewSeverity is intentionally separated from the lint Severity so migration safety can be reasoned about independently from schema quality.
Add positive and negative test cases for each rule in `risk/<id>` namespace, asserting both severity escalation and intentional-disconnect silencing behavior.
Wires `parse → diff → run_rules → suppression → summary` into a single `run_review` entry point with text / markdown / json output helpers. Handles `--rules` / `--except-rule` / `--except-table` / `--deny` semantics in the application layer.
Each rule gets a positive case; rules with notable false-positive boundaries get a paired negative fixture (FK also dropped, indexed FK, NOT NULL on a brand-new table). The integration test golden-compares findings against `expected.json`; set `UPDATE_FIXTURES=1` to refresh.
Wire up the relune-app review pipeline into a new `relune review` subcommand with --before/--after inputs (file/text/JSON variants), --format text|markdown|json, --rules/--except-rule/--except-table, --deny <severity>, --exit-code, and -o/--out support.
Add a [review] table to ReluneConfig with format, dialect, rules, except_rules, except_tables, and deny fields, plus the merge_review_args helper used by the review command. CLI flags continue to take precedence over config values.
Cover text, markdown, and JSON formatting paths along with --deny, --exit-code, --except-rule, --except-table, --out, and config-driven defaults. Capture the review --help output as an insta snapshot so the CLI surface stays stable.
Describe the review CLI surface, configuration keys, and crate roles so docs/cli-reference.md, docs/configuration.md, and ARCHITECTURE.md cover the new schema review workflow.
Promote the new schema review workflow alongside lint and diff in the top-level README and the relune skill so quick-start examples and migration-review playbooks include the new command.
…tries
Inline `UNIQUE` column options and `UNIQUE` table constraints were previously
discarded during CREATE TABLE parsing, which left review rules unable to detect
that a column drop also removes a unique guarantee. Materialize both forms as
`Index { is_unique: true }` entries and dedupe by name + column set so the
schema model reflects every uniqueness contract.
…tion `risk/drop-pk-or-unique` now escalates the UNIQUE-index removal path to `breaking` when an incoming foreign key targets the unique column set, mirroring the primary-key escalation rule. `risk/drop-column-referenced` no longer skips same-table foreign keys, so self-referencing FK target columns are reported correctly. The same-table overlap with the outgoing-FK pass is filtered out to avoid duplicate findings.
- Add a dedicated `ReviewDenied` `CliError` variant that maps to exit code `10`, matching the documented "review denied" gate semantics so CI can distinguish a deny-threshold failure from a generic error. - Honour `--quiet` for `review` by emitting only the summary line in text and markdown formats; the findings body and per-rule details are suppressed when the flag is set. - Update review CLI smoke tests: rename the deny test to assert exit code 10 and add a `--quiet` suppression test.
`risk/drop-pk-or-unique` previously missed the case where a primary key is widened in place — e.g. `PRIMARY KEY (id)` becoming `PRIMARY KEY (id, tenant_id)` — because no individual column loses its PK status, so the column-path check is never entered. The original column set is no longer guaranteed unique once the PK is composite, and an FK that still references just `(id)` is unsupportable in most engines. Add a per-table check that fires when the after PK is a strict superset of the before PK and no UNIQUE index in `after` covers the original column set. The severity escalates to `breaking` when an incoming FK targets the exact original column set, mirroring the existing PK-drop and UNIQUE-drop escalations. Tighten the column-path equality check to use exact set comparison rather than superset coverage as a defensive measure.
Code Metrics Report
Details | | main (2a4d39f) | #91 (3dae7e9) | +/- |
|---------------------|----------------|---------------|-------|
- | Coverage | 94.7% | 94.6% | -0.2% |
| Files | 77 | 81 | +4 |
| Lines | 34529 | 36827 | +2298 |
+ | Covered | 32726 | 34864 | +2138 |
- | Test Execution Time | 1m26s | 1m32s | +6s |Code coverage of files in pull request scope (93.3% → 93.2%)
Reported by octocov |
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
1 participant
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.
Summary
relune reviewsubcommand that diffs two SQL/JSON inputs and emits migration risk findings classified asinfo | warning | caution | breaking.relune-core::reviewengine implementing eight built-in rules:risk/drop-column-referenced,risk/drop-table-referenced,risk/drop-pk-or-unique,risk/add-not-null-on-existing,risk/type-narrow,risk/fk-without-index,risk/add-unique-on-existing,risk/add-cascade-delete.SchemaDiffwithon_delete/on_updateaction diffs so cascade-related rules can reason about FK action changes.--rules/--except-rule/--except-tablefiltering,--deny <severity>gating with exit code10, and a[review]section inrelune.toml.UNIQUEcolumn options and table-levelUNIQUEconstraints asIndexentries so review rules can reason about uniqueness regardless of declaration form.Changes
ReviewSeverity,ReviewRuleId(withrisk/*serde namespace),RiskFinding, and therun_rulesorchestrator that runs each rule against the diff plus the before/after schemas.breaking, FK without supporting index isinfo).ReviewRequest/ReviewResultand provide text / markdown / JSON formatters that include summary counts and per-rule findings.fixtures/review/<rule-name>/{before.sql,after.sql,expected.json}cases plus paired safe/unsafe variants for the rules with severity escalation logic.relune reviewthrough the CLI with--before/--afterselectors, format flag, output path, and--rules/--except-rule/--except-tablefiltering.relune.tomland merged with command-line flags.ColumnOption::UniqueandTableConstraint::UniqueintoIndex { is_unique: true }entries so review rules can detect uniqueness loss; dedupe by name and column set.risk/drop-pk-or-uniquetobreakingwhen an incoming FK references the dropped UNIQUE column set, mirroring the existing PK escalation.risk/drop-column-referencedso self-referencing FK target columns are reported, while filtering the overlap with the outgoing-FK pass to avoid duplicate findings.ReviewDeniedCliErrorvariant that maps to exit code10, distinguishing a deny-threshold failure from a generic error.--quietforreviewby emitting only the summary line in text and markdown formats.aftercovers the original column set; severity escalates tobreakingwhen an incoming FK targets the original column set.