Renew metric report interface#39
Merged
Merged
Conversation
- Add metric-native measurements and snapshot summaries - Keep legacy JSON and CSV output paths for existing integrations - Document AI-assisted review workflows
- Move RuboCop calculator calls behind a standard engine adapter - Keep metric classes focused on scope extraction and measurement shaping
- Explain how CodeKeeper uses RuboCop analysis without applying offense policy - Document metric-native scopes and AI review usage
- Drop legacy metric engine and legacy output aliases - Keep score compatibility views derived from metric measurements - Remove redundant direct dependencies and align Ruby support with maintained versions
Use MetricReport for the full metrics output object while keeping summary as the derived aggregate section.
Require metric classes to receive SourceFile instances and document that RuboCop output is threshold-gated and policy-filtered.
ebihara99999
changed the title
26 tasks
Limit each metric summary to the top five measurements by value and make tie ordering stable for repeatable reports.
Add csv as a runtime dependency and align hotspot tie-break tests with the documented ordering.
The gemspec now declares csv as a runtime dependency, which the gemspec directive already brings into the bundle on every Ruby version, so the conditional entry duplicated it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the Parser facade and the per-metric score hash interface, which predate the metric report model. SourceFile is now the single parsing boundary and Measurement the only metric output. Also remove RuboCop mixins and a version fallback made unreachable by the centralized calculator and the rubocop >= 1.88 requirement, and move file loading coverage from the parser specs to the SourceFile specs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AbcMetric and CyclomaticComplexity intentionally share an identical method-scope enumerator. Record that the duplication is deliberate and that a shared enumerator should be extracted when a third method-scope metric is added, so the decision survives beyond the current work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measurements are the source of truth, and the CSV view flattened them into metric/key/value rows that silently overwrote measurements sharing a key, such as classes reopened across files. It was also the only reason for the csv runtime dependency and the score key rules on Measurement. Tabular views can be derived from the measurements array downstream, so drop the csv format together with the format setting, the Result wrapper, and the csv dependency. Scorer now dedupes configured metrics itself, which the removed scores hash did implicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scope names are the identity of a measurement across runs, so they must name the owner precisely. Two defects broke that. class << self and the defs inside it were reported as "class << self" and instance-style names, hiding the owner and colliding across files. Resolve self to the enclosing constant, but only when the singleton class sits directly in a class or module body; inside method bodies and blocks self is the runtime receiver and stays unresolved. defs and literal define_methods directly in a singleton class body are named as singleton methods (Owner.method), matching def self.method. owner_name qualified each ancestor recursively before joining, so nested namespaces duplicated exponentially (Gitlab::Gitlab::Ci::... for lib/gitlab/ci files). Join the raw constant sources instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gem build writes the package to the repository root, where git ls-files based packaging in the gemspec could pick it up as a tracked file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Scope discovery approximated Ruby assignment and self semantics case by case, which left false positives and false negatives at the edges. Replace the approximations with two rules closed over static decidability. Singleton classes: self identifies the enclosing constant only directly inside a class or module body, so class << self there is measured and resolved (class << Foo), while method bodies, blocks, and the top level keep self unresolved and are not measured as class_length scopes. Constant receivers are static anywhere; other receivers never are. The same rule now applies to def self.* naming. Constant assignments: multiple assignment resolves through nested destructuring by object identity, arrays assign by position, a splat leaves only the fixed prefix and suffix statically determined, and any other right-hand side goes to the first fixed position only. The resolution moved to ConstantAssignment, which also keeps ClassLength under its own class length metric. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Looks good. |
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.
Background
CodeKeeper 1.0 renews the tool around a metric-native
MetricReportdesigned for recurring human and AI-assisted code quality review. RuboCop offense output is threshold-gated and policy-filtered, so it cannot serve as a metric inventory; this PR makes CodeKeeper emit the full inventory instead. Design decisions and acceptance criteria: #40.Changes
MeasurementandMetricReportas the public output model. JSON output containsmeasurementsas the source of truth andsummaryderived from them;top_hotspotslists up to five measurements per metric in descending value order with deterministic ties by path, start line, and scope name..rubocop.yml, inline disable comments,Max, andAllowed*never suppress or alter measurements.class << selfwith its methods resolves to the enclosing constant (class << Foo,Foo.bar) only directly inside a class or module body. In method bodies, blocks, and at the top level,selfstays unresolved.ConstantAssignment).Parser, the per-metricscoreinterfaces, thescoreshash, CSV output, andconfig.format.SourceFileis the parsing boundary andScorer.keepreturns aMetricReport.Breaking changes
config.formatare removed. Tabular views can be derived from themeasurementsarray; settingconfig.formatnow raisesNoMethodError.Verification
bundle exec rspec: 47 examples, 0 failures.bundle exec rubocop: no offenses. CI green on Ruby 3.3, 3.4, and 4.0.85ed8239(app/+lib/, 11,355 files) plus the Ruby 4.0.3 standard library and installed gems (12,623 files): no crashes and no invariant violations (JSON round-trip, summary recomputation, abc/cyclomatic count equality, line bounds), with byte-identical output for 1 and 8 threads.app/modelswith a forced configuration and disable comments ignored: 19,476 of 19,479 offenses match CodeKeeper measurements exactly by position and value. The remaining differences are the documented singleton-class policy (Add corpus-based verification for the metric report #41).Review notes
ScopeName,ConstantAssignment); every rule is pinned by fixtures underspec/fixtures/class_samples/.