Skip to content

WA-VERIFY-048: Add script/rails7_migration_patterns_check#1123

Open
kitcommerce wants to merge 1 commit intonextfrom
issue-912-verify-048-migration-doc-check
Open

WA-VERIFY-048: Add script/rails7_migration_patterns_check#1123
kitcommerce wants to merge 1 commit intonextfrom
issue-912-verify-048-migration-doc-check

Conversation

@kitcommerce
Copy link
Contributor

Summary

Closes #912

Adds script/rails7_migration_patterns_check, a read-only Ruby script that validates each Rails 7 migration pattern doc contains the four required structural headings.

What it does

  • Scans docs/rails7-migration-patterns/*.md (excluding README.md)
  • For each file, checks for headings matching: Symptom, Root cause, Detection, Fix (case-insensitive, any heading level)
  • Exits 0 with a summary when all docs pass
  • Exits 1 and lists the file(s) + missing sections when any are incomplete
  • Makes no modifications to docs — purely validation

Client impact

None (developer tooling only)

Verification Plan

# From repo root
./script/rails7_migration_patterns_check

Expected output (current state): exits 1, listing the 7 docs that are missing required sections — this confirms the script correctly identifies structural gaps in the existing docs.

To verify a passing run: add the four required headings to one of the listed files temporarily, re-run, and confirm that file no longer appears in the output.

To verify the script is read-only: inspect git status after running — no files should be modified.

…e validation

Closes #912 (WA-VERIFY-048)

Adds a read-only Ruby script that scans docs/rails7-migration-patterns/*.md
(excluding README.md) and verifies each file contains headings for the four
required sections: Symptom, Root cause, Detection, Fix.

- Exits 0 with a summary when all docs pass
- Exits 1 and lists file(s) + missing sections when structure is incomplete
- No modifications to docs; purely a validation tool
@kitcommerce kitcommerce added gate:build-pending Build gate running gate:build-passed Build gate passed review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress and removed gate:build-pending Build gate running labels Mar 17, 2026
@kitcommerce
Copy link
Contributor Author

Simplicity Review

Verdict: PASS_WITH_NOTES (LOW)

Findings

  • The script is appropriately small (54 lines) and does exactly one thing: validate that Markdown docs contain required headings. That's the right scope for a utility script.
  • The split between REQUIRED_SECTIONS and REQUIRED_SECTIONS_EXTRA with an immediate ALL_REQUIRED merge is needlessly convoluted. If all four sections are required, just define one constant. The two-constant pattern implies they are treated differently somewhere — but they are not.
  • Naming REQUIRED_SECTIONS_EXTRA is particularly unclear. "Extra" relative to what? The name implies optional or supplementary, but the code treats them as mandatory.

Recommendations

  • Collapse to a single constant:
    REQUIRED_SECTIONS = %w[Symptom Detection Fix].freeze  # remove this split
    ALL_REQUIRED = %w[Symptom Detection Fix Root\ cause].freeze
    Or more directly:
    REQUIRED_SECTIONS = ['Symptom', 'Root cause', 'Detection', 'Fix'].freeze
  • No other structural concerns. The glob + reject + sort pattern is clean and idiomatic.

@kitcommerce
Copy link
Contributor Author

Architecture Review

Verdict: PASS

Findings

  • Scope is well-contained. A standalone 54-line Ruby script in script/ with no runtime dependencies beyond stdlib — zero coupling to the application domain, persistence, or UI layers. This is textbook separation of concerns for developer tooling.
  • Dependency direction is correct. The script reads docs via filesystem glob; docs know nothing about the script. No circular or inward-pointing dependencies introduced.
  • Public surface area unchanged. No new modules, classes, or APIs added to the application. The script is a leaf node in the dependency graph.
  • Convention-consistent placement. script/ is the standard Rails location for project utilities. Follows existing patterns.
  • Required-sections contract is explicit. REQUIRED_SECTIONS + REQUIRED_SECTIONS_EXTRAALL_REQUIRED makes the heading contract clear and easy to extend. Minor nit: two constants could be one flat array, but the separation (single-word vs multi-word headings) is a reasonable readability choice, not an architectural concern.

Recommendations

  • None blocking. If the doc structure contract evolves (e.g., optional sections, severity tiers), consider extracting the heading list to a shared YAML config so both the check script and any doc generators stay in sync. Not needed today — just a future extensibility note.

Reviewed as part of WA-VERIFY-048. No architectural concerns.

@kitcommerce
Copy link
Contributor Author

Security Review

Verdict: PASS

Findings

No security issues identified. This is a 54-line read-only validation script that:

  • Reads local markdown files from a known directory (docs/rails7-migration-patterns/)
  • Uses Regexp.escape correctly when building patterns from constants
  • Contains no network calls, no secret handling, no user-supplied input
  • Has no deserialization, no shell-outs, no dynamic code execution

Recommendations

None — clean from a security perspective.


🔒 Automated security review — Kit

@kitcommerce
Copy link
Contributor Author

Rails Conventions Review

Verdict: PASS_WITH_NOTES

Findings

  • script/rails7_migration_patterns_check:17–19REQUIRED_SECTIONS + REQUIRED_SECTIONS_EXTRA split exists solely because "Root cause" has a space, then they're immediately merged into ALL_REQUIRED. This is unnecessarily convoluted; just define ALL_REQUIRED directly as a single array of all four strings. The comment explaining the workaround is a smell that the abstraction wasn't needed.

  • script/rails7_migration_patterns_check:22 — Long one-liner chaining Dir.glob, reject, and sort on a single line. Rails style (and standard Ruby) favours extracting this or at minimum breaking it across lines for readability. Not a blocker, but worth a quick split.

  • script/rails7_migration_patterns_check (general) — No executable bit note in the PR, but the diff shows new file mode 100755 — looks correct. Confirming the shebang (#!/usr/bin/env ruby) is present and the frozen_string_literal magic comment is used — both are good Rails-script conventions. ✅

What's Good

  • Read-only, no side effects, no gem dependencies — exactly the right shape for a script/ utility.
  • Regexp.escape used correctly — no injection risk.
  • UTF-8 encoding specified explicitly on File.read — correct defensive practice.
  • Exit codes documented in the header comment — developer-friendly.
  • File.expand_path("../docs/...", __dir__) is the idiomatic Rails script path resolution pattern. ✅

Recommendations

  1. Collapse REQUIRED_SECTIONS, REQUIRED_SECTIONS_EXTRA, and ALL_REQUIRED into a single constant.
  2. (Optional) Break the Dir.glob + reject + sort chain across two lines.

Neither finding affects correctness or the Rails convention score meaningfully — this is dev tooling, not application code. The core logic is clean and idiomatic Ruby. Approving with the above notes for the author's consideration.

@kitcommerce kitcommerce added review:architecture-done Review complete review:simplicity-done Review complete review:security-done Review complete review:rails-conventions-done Rails conventions review complete review:wave1-complete review:rails-security-pending Rails security review in progress review:database-pending Database review in progress review:test-quality-pending Review in progress and removed review:architecture-pending Review in progress review:simplicity-pending Review in progress review:security-pending Review in progress review:rails-conventions-pending Rails conventions review in progress labels Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate:build-passed Build gate passed review:architecture-done Review complete review:database-pending Database review in progress review:rails-conventions-done Rails conventions review complete review:rails-security-pending Rails security review in progress review:security-done Review complete review:simplicity-done Review complete review:test-quality-pending Review in progress review:wave1-complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant