refactor: architecture cleanup — autoload, dimensions, pipeline, type safety#153
Merged
Conversation
… safety
Consolidates four major architectural improvements:
1. **Autoload migration (D11)**: Replace all internal `require_relative`
with Ruby `autoload` declared in immediate parent namespace files.
Added namespace files for Canon::{Xml,Html,Diff,Formatters,Validators,
PrettyPrinter,Options,Commands,TreeDiff::*} with all children autoloaded.
Zero `require_relative` calls remain in lib/canon/.
2. **Dimension architecture (D04)**: Replace 8 dimension classes
(BaseDimension + 7 subclasses) with a single Dimension value object
and DimensionSet. Comparison logic lives in comparators where it has
full node context. Registry provides pre-built sets per format
(XML/HTML share 7, JSON has 3, YAML has 4).
3. **Shared pipeline + DiffNodeBuilder (D07, D09)**: Extract Pipeline
module consolidating shared steps between DOM and Semantic diff
(format detection, validation, config merge, parsing). DiffNodeBuilder
is the single DiffNode factory for the DOM path.
4. **ConfigDSL + type safety (D10, D12, D13)**: ConfigDSL eliminates
5-line getter/setter boilerplate per config attribute. Eliminate
`respond_to?` and `instance_variable_set` throughout. Fix
NodeInspector namespace typo (D14).
Also:
- Add comprehensive specs for Pipeline, ConfigDSL, and Dimensions
- Auto-correct rubocop style offenses
- Update .rubocop_todo.yml
All 2411 specs pass; rubocop clean.
Auto-correct rubocop 1.87 Performance/StringInclude offenses in spec
files: replace .match(/literal/) with .include?('literal'). Regenerate
.rubocop_todo.yml against rubocop 1.87.
moxml 0.1.24+ autoloads Moxml::Adapter::Base from lib/moxml.rb instead of eager require_relative in adapter/rexml.rb. Loading adapter/rexml directly without first loading the moxml namespace leaves Base undefined under Opal, breaking the smoke test suite. Prepending 'moxml' to runner.requires sets up the autoload chain before the Rexml adapter references Base.
The previous performance comparator loaded both branches' Canon code into a single Ruby process via module_eval, relying on $LOAD_PATH tricks to switch between them. Canon's move to autoload exposed the flaw: autoload resolution is global, so constants resolved to whatever lib path sat at the front of $LOAD_PATH, mixing files from both branches and producing NameError when base-main's html_comparator referenced Canon::Comparison::MarkupComparator before it was defined. The new design runs the benchmark suite in two separate Ruby subprocesses — one per branch — each loading its own Canon from disk with full isolation. Each subprocess emits JSON to stdout via the new lib/tasks/performance_report.rb entry point; the comparator diffs the two payloads and reports regressions. Side fixes: - Lower-bound IPS clamp prevents nonsensical negative values when variance exceeds the mean for very fast benchmarks. - Drops broken performance:json and performance:yaml rake tasks that used $send to call a non-existent private method.
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.
Consolidates four major architectural improvements (TODO.dims items D04, D07, D09–D14) into one cohesive refactor.
Changes
1. Autoload migration (D11)
Replace all internal
require_relativewith Rubyautoloaddeclared in immediate parent namespace files.Canon::{Xml, Html, Diff, Formatters, Validators, PrettyPrinter, Options, Commands}andCanon::TreeDiff::*sub-namespaces — each declares autoloads for its direct children.lib/canon.rbnow usesrequire "canon/..."(load-path style) for bootstrap.class Canon::Configdeclares autoloads forConfigDSL,EnvProvider,EnvSchema,OverrideResolver,ProfileLoader,TypeConverter.require_relativecalls remain inlib/canon/(only documentation mentions in comments).2. Dimension architecture (D04)
Replace 8 dimension classes (
BaseDimension+ 7 subclasses) with a singleDimensionvalue object andDimensionSet.Registryprovides pre-built sets per format: XML/HTML share 7 dimensions, JSON has 3, YAML has 4. Format aliases (html, html4, html5) all resolve to the XML set.3. Shared pipeline + DiffNodeBuilder (D07, D09)
Pipelinemodule consolidates shared steps between DOM and Semantic diff (format detection, validation, config merge, parsing) — keeps the two algorithm cores independent while preventing drift in pipeline mechanics.DiffNodeBuilderis the singleDiffNodefactory for the DOM comparison path.4. ConfigDSL + type safety (D10, D12, D13, D14)
ConfigDSLeliminates the 5-line getter/setter boilerplate per config attribute; provides single source of truth for attribute types/enums/defaults.respond_to?throughout — replaced withis_a?type checks or proper polymorphism.instance_variable_setinConfigDSL(usedclass_variable_defined?+ class-variable access).NodeInspectornamespace typo.5. New specs
spec/canon/comparison/pipeline_spec.rb— Pipeline module coverage.spec/canon/config/config_dsl_spec.rb— ConfigDSL coverage.spec/canon/comparison/dimensions/—Dimension,DimensionSet,Registry.6. Housekeeping
.rubocop_todo.yml.Test results
bundle exec rubocopcleanArchitectural compliance
require_relativein lib/canon/ (autoload only)respond_to?for type checkinginstance_variable_set/getsendto bypass visibility