Phase 1b: Extract analyses, QueryRunner, QueryExplainer + 0.4.0 paired release#7
Merged
Merged
Conversation
Thirteen-task plan extracting the 5 database analyses, Core::QueryRunner, and Core::QueryExplainer from the Rails controller concerns into mysql_genius-core, then doing the paired release of mysql_genius-core 0.1.0 + mysql_genius 0.4.0 to rubygems. Stage A extracts the 5 analyses one per task (TableSizes, DuplicateIndexes, QueryStats, UnusedIndexes, ServerOverview) using the same TDD pattern: spec with FakeAdapter, implementation, require wiring, concern delegation, suite verification, commit. Stage B adds Core::ExecutionResult (value object), Core::QueryRunner:: Config (keyword-init Struct), Core::QueryRunner (owns SQL validation, row-limit/timeout-hint application, execution, column masking, timeout detection), and Core::QueryExplainer. Audit logging stays in the Rails concern. Stage C updates .github/workflows/publish.yml to build and push both gems in the correct order (core first so the dependency resolves at install time), bumps versions, updates the gemspec pin from "~> 0.1.0.pre" to "~> 0.1", flips the CHANGELOG Unreleased section to 0.4.0, merges Phase 1b via PR, tags v0.4.0, verifies the release. Stage D is final verification: both suites green, both gems on rubygems, fresh install resolves transitively. Two intentional scope exclusions vs spec: the 7 inline AI features in AiFeatures concern stay in the adapter (they already delegate through Core::Ai::Client via the Phase 1a helper and aren't analyses-on-a- connection), and ERB template extraction stays deferred to Phase 2 alongside a concrete non-Rails consumer.
The superpowers design specs and implementation plans live in docs/superpowers/ as local planning artifacts and aren't meant to be shipped as part of the gem or referenced from the user-facing codebase. Add the directory to .gitignore and remove the existing tracked files from the index. The files remain on disk for local reference; they just won't be committed going forward. Historical commits on main and earlier on this branch still contain these files — this is a going-forward cleanup, not a history rewrite.
Move the information_schema.tables query and result transformation out of the DatabaseAnalysis concern into a new MysqlGenius::Core::Analysis::TableSizes class that takes a Core::Connection. The concern's table_sizes action now builds an ActiveRecordAdapter and delegates. Behavior is byte-identical — same SQL, same result shape, same per-table COUNT(*) fallback on error, same needs_optimize threshold. The 5 new core specs use FakeAdapter to stub information_schema results and verify row handling (hash access, MariaDB uppercase column names, COUNT(*) error fallback, needs_optimize computation).
Code-review feedback on Task 1 of Phase 1b (commit 673c144): 1. RuboCop was regressing against the Phase 1a baseline. Fixed via `rubocop -A`: - RSpec/LeadingSubject: subject declared before let - RSpec/EmptyLineAfterFinalLet: blank line before subject - Style/WordArray: replaced %w[table_name ...] with string array literals (rubocop-shopify rejects %w containing underscores) 2. RSpec/ExampleLength: split the 31-line "hash per table" example into two smaller examples under the project's 25-line cap: - "returns one hash per BASE TABLE row" (top-level shape) - "includes full size metadata for each row" (numeric keys) 3. Added two coverage gap tests flagged by the reviewer: - nil size columns (e.g., MEMORY engine tables) coerce to 0.0 - exact 10% boundary does not trigger needs_optimize
Move the left-prefix duplicate-index detection out of the DatabaseAnalysis concern into a new MysqlGenius::Core::Analysis::DuplicateIndexes class. The class takes a Core::Connection plus a blocked_tables list and returns an array of duplicate pairs, with the symmetrical-coverage dedup preserved. The concern now builds an ActiveRecordAdapter and delegates, passing blocked_tables from the configuration.
Move the performance_schema.events_statements_summary_by_digest query out of the DatabaseAnalysis concern into a new MysqlGenius::Core::Analysis::QueryStats class. Sort whitelist and limit clamping now live in the class as VALID_SORTS and MAX_LIMIT constants; the concern action parses params and hands them to .call(sort:, limit:). performance_schema availability errors continue to be caught by the concern (Rails-specific ActiveRecord::StatementInvalid) and rendered with the same user-facing message as before.
Move the performance_schema.table_io_waits_summary_by_index_usage JOIN query out of the DatabaseAnalysis concern into a new MysqlGenius::Core::Analysis::UnusedIndexes class. The class takes a Core::Connection and returns an array of hashes with a ready-to-run ALTER TABLE DROP INDEX statement per entry. performance_schema availability errors stay in the concern's ActiveRecord::StatementInvalid rescue.
Move SHOW GLOBAL STATUS / SHOW GLOBAL VARIABLES / SELECT VERSION() collection and derived-metric computation out of the DatabaseAnalysis concern into a new MysqlGenius::Core::Analysis::ServerOverview class. Structure the output into server/connections/innodb/queries blocks with the same keys and types as before. This completes the 5-analysis extraction. DatabaseAnalysis concern is now fully delegated to core; the concern's 5 actions have shrunk from 295 lines of inline SQL and transformations to 47 lines of thin delegating wrappers.
Immutable frozen value object returned from Core::QueryRunner#run (to be added in the next task). Contains columns, rows (post-masking), row_count, execution_time_ms, and a truncated flag. Distinct from Core::Result because it carries runtime metadata that plain results don't need.
Holds the runner-specific subset of MysqlGenius::Configuration: blocked_tables, masked_column_patterns, query_timeout_ms. Frozen on initialize for defensive immutability. Forward-declares the QueryRunner class so Config can be namespaced under it. The full QueryRunner implementation lands in the next task.
Core::QueryRunner owns SQL validation, row-limit application, timeout-hint wrapping (MySQL vs MariaDB flavors), execution, column masking, and timeout detection. Returns a Core::ExecutionResult or raises Rejected / Timeout. Does NOT handle audit logging — the concern still owns that. The execute action in the QueryExecution concern now parses params, builds an ActiveRecordAdapter and a QueryRunner::Config, calls runner.run, and catches the specific error classes to produce the same JSON response shapes as before. Rails-specific ActiveRecord error translation stays in the concern. Helpers validate_sql / apply_timeout_hint / mariadb? / apply_row_limit / timeout_error? / masked_column? stay in the concern temporarily because explain still uses validate_sql — they're removed in Task 9 when QueryExplainer replaces that last consumer. 13 new core specs cover happy-path execution, each rejection reason, row limit application, column masking (single and multiple patterns), truncation detection, MySQL vs MariaDB timeout hints, timeout detection via error message, and non-timeout error propagation.
Core::QueryExplainer wraps EXPLAIN around validated SELECT queries, with an optional skip_validation flag for explaining captured slow queries from mysql's own logs. Reuses Core::QueryRunner::Rejected for validation failures and adds a new Core::QueryExplainer::Truncated class for SQL that appears cut mid-statement. The explain action in the QueryExecution concern now builds an ActiveRecordAdapter + Config and delegates. The concern's private helpers validate_sql, apply_timeout_hint, mariadb?, apply_row_limit, timeout_error?, and masked_column? are deleted — they were the internal plumbing that is now owned by Core::QueryRunner and Core::QueryExplainer. sanitize_ai_sql and audit remain because they're used elsewhere in the controller. This completes Stage B and all code extraction for Phase 1b. The next stage is the paired release to rubygems.
The workflow now builds and publishes mysql_genius-core first, then mysql_genius, so the runtime dependency on mysql_genius-core resolves at gem install time. The test job also runs the core gem's spec suite to ensure it passes before publishing anything. working-directory is used to keep the core gem's build artifacts in gems/mysql_genius-core/ so the root-level "gem push mysql_genius-*.gem" glob matches only the Rails adapter gem and not the core gem.
- mysql_genius-core: 0.1.0.pre -> 0.4.0 - mysql_genius: 0.3.2 -> 0.4.0 - mysql_genius.gemspec dep: "~> 0.1.0.pre" -> "~> 0.4.0" - CHANGELOG.md: ## Unreleased -> ## 0.4.0 with Phase 1b additions - Added gems/mysql_genius-core/CHANGELOG.md documenting the first published release of the core gem mysql_genius-core jumps straight from 0.1.0.pre to 0.4.0 so that both gems release in lockstep under matching version numbers going forward. The gemspec dep uses "~> 0.4.0" (allowing 0.4.x patches but not 0.5) to enforce exact-minor pairing. Also fixed the core gemspec: - changelog_uri now points at gems/mysql_genius-core/CHANGELOG.md (was pointing at the adapter's CHANGELOG at the repo root) - spec.files now includes CHANGELOG.md in the packaged gem The next commit tags v0.4.0 and kicks off the publish workflow, which will push mysql_genius-core 0.4.0 first and then mysql_genius 0.4.0 to rubygems.
spec.files used git ls-files filtered only on ^(test|spec|features)/, which meant every release of the mysql_genius gem was bundling a dormant copy of the entire gems/mysql_genius-core/ subtree (47 files, ~150KB) into its package. Those files aren't on the gem's require_paths (["lib"] only), so nothing broke at load time — it was just dead weight shipped in every release and a drift-reading hazard for downstream users who might think the bundled copy is live. mysql_genius-core is published as its own gem now; the adapter picks it up as a normal runtime dependency. Verified: tar -tzf mysql_genius-0.4.0.gem shows 0 files under gems/ after the fix, down from 47.
Core::Connection::ActiveRecordAdapter was added in Phase 1a but
never wired into any production require chain. On main it was
latent because no production code referenced it. Phase 1b's
extracted delegators instantiate it in every DatabaseAnalysis /
QueryExecution / AiFeatures concern action, so the missing require
surfaces as:
uninitialized constant MysqlGenius::Core::Connection::ActiveRecordAdapter
on the first request to a real host app — breaking tables, query
stats, unused indexes, duplicate indexes, server overview, execute,
explain, AI suggest, and AI optimize. Every tab except slow_queries.
Root cause: lib/mysql_genius.rb required mysql_genius/core (which
loads FakeAdapter from the core gem) but not the adapter-side file
at lib/mysql_genius/core/connection/active_record_adapter.rb. The
adapter spec file did its own explicit require, so the unit suite
was green and CI never saw the bug. Only a real Rails host-app boot
exercises the missing require.
Fix:
- lib/mysql_genius.rb now requires mysql_genius/core/connection/
active_record_adapter right after mysql_genius/core, so any
consumer doing `require "mysql_genius"` gets the full constant
namespace the concerns reference.
- spec/spec_helper.rb has a new boot-order regression guard that
raises at spec_helper load time if Core::Connection::
ActiveRecordAdapter is not reachable after `require "mysql_genius"`.
Placed in spec_helper rather than an RSpec example because by the
time individual spec files run, other spec files may have done
their own explicit requires and masked the bug. Verified the guard
catches the regression by temporarily removing the require and
watching `bundle exec rspec` abort with the error message.
- spec/mysql_genius/core/connection/active_record_adapter_spec.rb no
longer needs its explicit `require "mysql_genius/core/connection/
active_record_adapter"` — spec_helper now guarantees the constant
is loaded. Removing it keeps the invariant tight: any spec file
that still needs an explicit require is a signal the production
require chain is incomplete.
- CHANGELOG.md 0.4.0 section gets a new ### Fixed entry documenting
the incident so anyone on an affected pre-release install knows
what happened.
Repro (before fix):
$ ruby -Ilib -Igems/mysql_genius-core/lib -r mysql_genius \
-e 'p defined?(MysqlGenius::Core::Connection::ActiveRecordAdapter)'
nil
After fix:
$ ruby -Ilib -Igems/mysql_genius-core/lib -r mysql_genius \
-e 'p defined?(MysqlGenius::Core::Connection::ActiveRecordAdapter)'
"constant"
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
Completes Phase 1 of the desktop-app extraction by moving the 5 database analyses plus the query runner and query explainer out of the Rails controller concerns into
mysql_genius-core, then does the paired release ofmysql_genius-core 0.4.0+mysql_genius 0.4.0.Host apps see no public API change — this is entirely an internal refactor. After
bundle update, themysql_geniusgem will pull inmysql_genius-coreas a transitive dependency; no Gemfile changes are required.Versioning — lockstep 0.4.0
Per session decision,
mysql_genius-corejumps straight from0.1.0.preto0.4.0so that both gems release in lockstep under matching version numbers going forward. The gemspec pinsmysql_genius-coreat"~> 0.4.0"— pessimistic on the minor, allowing 0.4.x patches but blocking 0.5.0 — enforcing exact-minor pairing. One git tag (v0.4.0) covers both gems.What's included
Extractions (Stage A + B):
MysqlGenius::Core::Analysis::*(TableSizes, DuplicateIndexes, QueryStats, UnusedIndexes, ServerOverview)Core::QueryRunner+Core::QueryRunner::Config+Core::ExecutionResultCore::QueryExplainerDatabaseAnalysisconcern shrunk from ~295 lines to 47 lines of thin delegationQueryExecutionconcern'sexecute/explaindelegated to core; private helpers removedRelease prep (Stage C):
.github/workflows/publish.ymlrewritten to build and push both gems in order (core first so the dep resolves at install time), withworking-directoryisolating the core build to avoid glob collisionsmysql_genius-core→0.4.0,mysql_genius→0.4.0CHANGELOG.mdflipped from## Unreleased→## 0.4.0gems/mysql_genius-core/CHANGELOG.mddocumenting the first published core release"~> 0.1.0.pre"→"~> 0.4.0"changelog_urifixed to point at the nestedgems/mysql_genius-core/CHANGELOG.md(was pointing at the adapter's CHANGELOG)mysql_genius.gemspecspec.filesreject regex now excludesgems/so the adapter gem no longer bundles a dormant copy of the core gem subtree in its package (~150KB / 47 files removed per release)Test plan
bundle exec rspecat repo root — 36 examples, 0 failures(cd gems/mysql_genius-core && bundle exec rspec)— 166 examples, 0 failuresbundle exec rubocopat repo root — 68 files, clean(cd gems/mysql_genius-core && bundle exec rubocop)— 45 files, cleangem build mysql_genius.gemspecsucceeds; package no longer contains files undergems/v0.4.0, verify publish workflow publishes both gems to RubyGemsbundle installagainstgem "mysql_genius", "0.4.0"resolvesmysql_genius-core 0.4.0transitivelyWhat's next
Phase 2 opens a fresh branch to build
mysql_genius-desktop— a standalone Sinatra-hosted app using the samemysql_genius-corelibrary with a Trilogy adapter.See the design spec §9 for the full Phase 2+ roadmap.