Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/full.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ ignore:
- "**/*Test.swift"

# Ruby / Rails
- "**/*_spec.rb"
- "**/*_test.rb"
- "**/spec/**/*_spec.rb"
- "**/test/**/*_test.rb"
- "tmp/**"
- "log/**"
- "public/assets/**"
Expand Down
4 changes: 2 additions & 2 deletions examples/multi-repo.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ ignore:
- "**/*Tests.swift"
- "**/*Test.swift"
# Ruby / Rails
- "**/*_spec.rb"
- "**/*_test.rb"
- "**/spec/**/*_spec.rb"
- "**/test/**/*_test.rb"
- "tmp/**"
- "log/**"
- "public/assets/**"
Expand Down
4 changes: 2 additions & 2 deletions examples/ruby.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ ignore:
- ".git/**"
- ".enola/**"
# Tests
- "**/*_spec.rb"
- "**/*_test.rb"
- "**/spec/**/*_spec.rb"
- "**/test/**/*_test.rb"
- "spec/**"
- "test/**"
# Ruby / Rails
Expand Down
1 change: 1 addition & 0 deletions internal/cachecov/coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ var versionCoverage = map[int][]string{
94: {"TestExtractEndpointFacts_ConstantMethod"}, // Swift single-value (constant) method property
95: {"TestResolveImports_CouplingKindTagged", "TestResolveImports_ReferenceBeatsAssociation"}, // Ruby synthetic-edge coupling_kind prop + framework-const ignore list
96: {"TestAST_AssignedAndReturnedCallback", "TestAST_AssignedCallback_ShadowGuarded", "TestAST_ReturnedPlainVariable_NoPhantomRef", "TestAST_ReturnedForwardReference_Resolves", "TestAST_ShadowedLoopVarNotResolvedAsCall"}, // Python assignment/return value-refs + scope-wide (not just param) shadow guard
97: {"TestMatchAnyGlob_MidPatternDoublestar", "TestGolden"}, // Directory-scoped Ruby test globs: production *_ab_test.rb no longer deleted from the graph
}

func TestCacheVersionCoverage(t *testing.T) {
Expand Down
13 changes: 10 additions & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,15 @@ func Default() *Config {
"**/*.test.tsx",
"**/*.spec.ts",
"**/*.spec.tsx",
"**/*_spec.rb",
"**/*_test.rb",
// Ruby, unlike Go and TS, has no co-located test convention: RSpec
// requires spec/, Minitest defaults to test/. Demand the directory as
// well as the filename — a bare "**/*_test.rb" also swallows production
// code that merely ends in the token (a job named cache_warmup_ab_test.rb),
// deleting it from the graph.
// Keep in sync with TestGlobs below: a file that stops being a test must
// stop being ignored, or it is dropped without being recovered.
"**/spec/**/*_spec.rb",
"**/test/**/*_test.rb",
".enola/**",
// Build / cache artifacts. These are generated output (often transpiled
// JS, e.g. Next.js .next/) and must never be indexed as source — doing so
Expand Down Expand Up @@ -95,7 +102,7 @@ func Default() *Config {
// include test symbols — but the engine collects them separately for
// reference-only extraction so the dead-code detector can see that a
// production symbol is exercised by a test and not mis-report it as dead.
TestGlobs: []string{"**/*_spec.rb", "**/*_test.rb"},
TestGlobs: []string{"**/spec/**/*_spec.rb", "**/test/**/*_test.rb"},
Extractors: []string{"cpp", "go", "grpc", "java", "kotlin", "openapi", "php", "python", "typescript", "swift", "ruby"},
Explainers: []string{"cycles", "layers", "crossrepo", "coverage", "unused-routes", "god-class", "hotspots", "dependency-depth", "exported-surface", "complexity-outliers"},
Renderers: []string{"llm_context"},
Expand Down
8 changes: 7 additions & 1 deletion internal/engine/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,13 @@ import (
// assigned/iterated/aliased locals, not just parameters — so a loop var or local reusing a
// same-named top-level def's name no longer fabricates a same-module edge. Cached Python
// snapshots must re-extract.
const cacheVersion = "v96"
// v97: the Ruby ignore/test globs are directory-scoped ("**/spec/**/*_spec.rb" rather than
// "**/*_spec.rb"), so a production file whose basename merely ends in the token _test/_spec
// (a job named cache_warmup_ab_test.rb) is indexed as source instead of being ignored and
// misrouted to reference-only test-ref extraction. The glob matcher gained the
// "<prefix>/**/<fileglob>" form to express it. Cached Ruby snapshots must re-extract: the
// file set reaching the extractor changes.
const cacheVersion = "v97"

// extractorCache holds per-extractor facts keyed by a content hash of the files
// the extractor depends on. It is loaded from disk at the start of a snapshot and
Expand Down
116 changes: 65 additions & 51 deletions internal/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"path/filepath"
"runtime"
"runtime/debug"
"slices"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -499,11 +500,33 @@ func (e *Engine) matchesTestGlob(relPath string) bool {
return matchAnyGlob(filepath.ToSlash(relPath), e.cfg.TestGlobs)
}

// matchAnyGlob reports whether a forward-slash path matches any of the patterns,
// mirroring the "**/<name>/**", trailing-"/**", and "**/<glob>" handling of
// isIgnored so test-glob matching stays consistent with ignore matching.
// matchAnyGlob reports whether a forward-slash path matches any of the patterns.
// It is the single matcher behind both the ignore list and the test globs, so a
// file the two lists disagree about cannot exist: an ignored file that stops being
// a test necessarily stops being ignored. Supported forms:
//
// vendor/** anchored directory prefix
// **/build/** a directory named "build" at any depth
// **/*_test.go a basename glob at any depth
// **/spec/**/*_spec.rb a basename glob under a directory named "spec"
//
// The last form is the only one that constrains directory and filename together;
// see matchDirScopedGlob for why the Ruby test globs need it.
func matchAnyGlob(relPath string, patterns []string) bool {
for _, pattern := range patterns {
// "<prefix>/**/<fileglob>". Handled first and exclusively: the branches
// below would match such a pattern only when exactly one directory sits
// between prefix and file, which is an artifact of filepath.Match reading
// "**" as "*", not a rule anyone intended.
if i := strings.Index(pattern, "/**/"); i >= 0 {
prefix, fileGlob := pattern[:i], pattern[i+len("/**/"):]
if !strings.Contains(fileGlob, "/") {
if matchDirScopedGlob(relPath, prefix, fileGlob) {
return true
}
continue
}
}
if strings.HasPrefix(pattern, "**/") && strings.HasSuffix(pattern, "/**") {
seg := strings.TrimSuffix(strings.TrimPrefix(pattern, "**/"), "/**")
if seg != "" && !strings.Contains(seg, "/") {
Expand Down Expand Up @@ -536,56 +559,47 @@ func matchAnyGlob(relPath string, patterns []string) bool {
return false
}

// isIgnored checks whether a path matches any ignore pattern.
func (e *Engine) isIgnored(relPath string, isDir bool) bool {
// Normalize to forward slashes for matching
relPath = filepath.ToSlash(relPath)

for _, pattern := range e.cfg.Ignore {
// "**/<seg>/**" — ignore a directory named <seg> at ANY depth (and
// everything under it). The literal-prefix branch below cannot handle this
// because the leading "**/" is not a real path component; match by checking
// whether any path segment equals <seg>. Also covers the top-level case.
if strings.HasPrefix(pattern, "**/") && strings.HasSuffix(pattern, "/**") {
seg := strings.TrimSuffix(strings.TrimPrefix(pattern, "**/"), "/**")
if seg != "" && !strings.Contains(seg, "/") {
for _, part := range strings.Split(relPath, "/") {
if part == seg {
return true
}
}
}
}

// Handle directory-only patterns
if strings.HasSuffix(pattern, "/**") {
dirPrefix := strings.TrimSuffix(pattern, "/**")
if relPath == dirPrefix || strings.HasPrefix(relPath, dirPrefix+"/") {
return true
}
}

// Standard glob match
matched, err := filepath.Match(pattern, relPath)
if err == nil && matched {
return true
}

// Also try matching just the filename for patterns like **/*.go
if strings.HasPrefix(pattern, "**/") {
subPattern := strings.TrimPrefix(pattern, "**/")
matched, err = filepath.Match(subPattern, filepath.Base(relPath))
if err == nil && matched {
return true
}
// Also try the full relative path
matched, err = filepath.Match(subPattern, relPath)
if err == nil && matched {
return true
}
// matchDirScopedGlob reports whether relPath's basename matches fileGlob AND
// prefix names one of its ancestor directories ("**/<seg>" for a segment at any
// depth, otherwise an anchored literal path).
//
// A filename alone cannot classify a Ruby test. `lib/foo_test.rb` is one and
// `app/jobs/cache_warmup_ab_test.rb` is a production A/B-test job, yet both end in
// the token `test`; matching on the suffix deleted the latter from the graph
// entirely. Ruby settles it by convention — RSpec requires spec/, Minitest defaults
// to test/ — so the directory segment is the signal, and this predicate lets a
// single pattern demand both halves.
//
// Because every element of dirSegs is by construction an ancestor of the basename,
// segment equality alone places the file under the directory: no depth bookkeeping,
// and "spec/user_spec.rb" (zero intervening directories) falls out for free.
func matchDirScopedGlob(relPath, prefix, fileGlob string) bool {
segs := strings.Split(relPath, "/")
if len(segs) < 2 {
return false // no directory component, so no prefix can name an ancestor
}
dirSegs, base := segs[:len(segs)-1], segs[len(segs)-1]

if m, err := filepath.Match(fileGlob, base); err != nil || !m {
return false
}
if seg, ok := strings.CutPrefix(prefix, "**/"); ok {
if seg == "" || strings.Contains(seg, "/") {
return false
}
return slices.Contains(dirSegs, seg)
}
return false
if prefix == "**" {
return true // any directory
}
return strings.HasPrefix(relPath, prefix+"/")
}

// isIgnored checks whether a path matches any ignore pattern. isDir is unused: the
// patterns discriminate on shape, not on file type, and a directory that matches is
// pruned by the caller.
func (e *Engine) isIgnored(relPath string, isDir bool) bool {
return matchAnyGlob(filepath.ToSlash(relPath), e.cfg.Ignore)
}

// runExtractors detects applicable extractors and runs them. When cache is
Expand Down
115 changes: 115 additions & 0 deletions internal/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,121 @@ func TestIsIgnored(t *testing.T) {
}
}

// TestMatchAnyGlob_MidPatternDoublestar covers the "<prefix>/**/<fileglob>" form,
// which lets a pattern require BOTH a directory segment and a filename shape.
//
// The Ruby test globs need it. A bare "**/*_test.rb" is a filename-suffix match, so
// a production ActiveJob named cache_warmup_ab_test.rb was ignored AND routed to
// reference-only test-ref extraction — its class vanished from the graph. No
// filename-only rule can separate that file from lib/foo_test.rb: both end in the
// token "test". The directory segment is the only reliable signal, and Ruby supplies
// one (RSpec requires spec/, Minitest defaults to test/).
func TestMatchAnyGlob_MidPatternDoublestar(t *testing.T) {
rubyTestGlobs := []string{"**/spec/**/*_spec.rb", "**/test/**/*_test.rb"}

tests := []struct {
name string
relPath string
patterns []string
want bool
}{
{
// The reported bug: a production A/B-test job under app/jobs.
"production job whose name ends in _ab_test",
"app/jobs/reporting/cache_warmup_ab_test.rb",
rubyTestGlobs,
false,
},
{
"production model named ab_test",
"app/models/ab_test.rb",
rubyTestGlobs,
false,
},
{
// Zero intermediate directories. filepath.Match's "*" never crosses a
// separator, so the pre-existing "**/<glob>" branch could not match this.
"spec directly under spec/",
"spec/user_spec.rb",
rubyTestGlobs,
true,
},
{
"spec one level down",
"spec/services/report_worker_spec.rb",
rubyTestGlobs,
true,
},
{
"spec segment at any depth, several levels down",
"engines/billing/spec/models/nested/invoice_spec.rb",
rubyTestGlobs,
true,
},
{
"minitest file under test/",
"test/models/user_test.rb",
rubyTestGlobs,
true,
},
{
// The dir segment is present but the basename shape is wrong.
"support file under spec/ is not a spec",
"spec/rails_helper.rb",
rubyTestGlobs,
false,
},
{
// "spec" must be a DIRECTORY segment, not the basename stem.
"file named spec.rb outside a spec dir",
"app/models/spec.rb",
rubyTestGlobs,
false,
},
{
"anchored prefix form",
"spec/models/user_spec.rb",
[]string{"spec/**/*_spec.rb"},
true,
},
{
"anchored prefix form does not match a nested spec dir",
"engines/billing/spec/models/user_spec.rb",
[]string{"spec/**/*_spec.rb"},
false,
},
// The pre-existing pattern forms must keep their semantics — the new branch
// fires only on a literal "/**/" in the pattern, which none of them contain.
{
"**/build/** still matches a nested build dir",
"data/build/kspCaches/devDebug/Gen.kt",
[]string{"**/build/**"},
true,
},
{
"**/*_test.go still matches by filename at any depth",
"internal/pkg/foo_test.go",
[]string{"**/*_test.go"},
true,
},
{
"vendor/** still matches an anchored prefix",
"vendor/github.com/foo/bar.go",
[]string{"vendor/**"},
true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := matchAnyGlob(tt.relPath, tt.patterns); got != tt.want {
t.Errorf("matchAnyGlob(%q, %v) = %v, want %v",
tt.relPath, tt.patterns, got, tt.want)
}
})
}
}

func TestResolveFactFile_SingleRepo(t *testing.T) {
cfg := config.Default()
eng, _ := New(cfg)
Expand Down
4 changes: 4 additions & 0 deletions internal/engine/testdata/golden/ruby_sample.facts.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
{"kind":"dependency","name":"config -\u003e rails/all","file":"config/application.rb","line":5,"repo":"ruby_sample","props":{"language":"ruby","source":"external"},"relations":[{"kind":"imports","target":"rails/all"}]}
{"kind":"file_ref","name":"app/services/report_worker.rb","file":"app/services/report_worker.rb","repo":"ruby_sample","props":{"dynamic_send_prefixes":["report_"],"language":"ruby"}}
{"kind":"file_ref","name":"app/views/reports/show.html.erb","file":"app/views/reports/show.html.erb","repo":"ruby_sample","props":{"language":"ruby"},"relations":[{"kind":"calls","target":"ReportPresenter"},{"kind":"calls","target":"ReportPresenter.render_summary"},{"kind":"calls","target":"can_view_reports?"},{"kind":"calls","target":"current_user"},{"kind":"calls","target":"render_summary"}]}
{"kind":"module","name":"app/jobs","file":"app/jobs","repo":"ruby_sample","props":{"framework":"rails","language":"ruby","module_role":"unknown"}}
{"kind":"module","name":"app/models/concerns","file":"app/models/concerns","repo":"ruby_sample","props":{"framework":"rails","language":"ruby","module_role":"unknown"}}
{"kind":"module","name":"app/services","file":"app/services","repo":"ruby_sample","props":{"framework":"rails","language":"ruby","module_role":"unknown"}}
{"kind":"module","name":"config","file":"config","repo":"ruby_sample","props":{"framework":"rails","language":"ruby","module_role":"unknown"}}
Expand All @@ -16,6 +17,9 @@
{"kind":"route","name":"/session","file":"config/routes.rb","line":20,"repo":"ruby_sample","props":{"action":"create","framework":"rails","language":"ruby","method":"POST","resource":"session"},"relations":[{"kind":"declares","target":"config"}]}
{"kind":"route","name":"/session","file":"config/routes.rb","line":20,"repo":"ruby_sample","props":{"action":"destroy","framework":"rails","language":"ruby","method":"DELETE","resource":"session"},"relations":[{"kind":"declares","target":"config"}]}
{"kind":"route","name":"/session","file":"config/routes.rb","line":20,"repo":"ruby_sample","props":{"action":"show","framework":"rails","language":"ruby","method":"GET","resource":"session"},"relations":[{"kind":"declares","target":"config"}]}
{"kind":"symbol","name":"CacheWarmupABTest","file":"app/jobs/cache_warmup_ab_test.rb","line":11,"repo":"ruby_sample","props":{"exported":true,"framework":"rails","language":"ruby","superclass":"BaseWorker","symbol_kind":"class"},"relations":[{"kind":"declares","target":"app/jobs"},{"kind":"implements","target":"BaseWorker"}]}
{"kind":"symbol","name":"CacheWarmupABTest#notify","file":"app/jobs/cache_warmup_ab_test.rb","line":18,"repo":"ruby_sample","props":{"cyclomatic":1,"exported":false,"framework":"rails","language":"ruby","symbol_kind":"method"},"relations":[{"kind":"declares","target":"app/jobs"}]}
{"kind":"symbol","name":"CacheWarmupABTest#perform","file":"app/jobs/cache_warmup_ab_test.rb","line":12,"repo":"ruby_sample","props":{"cyclomatic":1,"exported":true,"framework":"rails","language":"ruby","symbol_kind":"method"},"relations":[{"kind":"calls","target":"notify"},{"kind":"declares","target":"app/jobs"}]}
{"kind":"symbol","name":"Reporting","file":"app/services/report_worker.rb","line":6,"repo":"ruby_sample","props":{"abstract":false,"exported":true,"framework":"rails","language":"ruby","symbol_kind":"interface"},"relations":[{"kind":"declares","target":"app/services"}]}
{"kind":"symbol","name":"Reporting::ReportWorker","file":"app/services/report_worker.rb","line":7,"repo":"ruby_sample","props":{"exported":true,"framework":"rails","language":"ruby","superclass":"BaseWorker","symbol_kind":"class"},"relations":[{"kind":"calls","target":"STOP_WORDS"},{"kind":"calls","target":"Trackable"},{"kind":"calls","target":"delegate"},{"kind":"calls","target":"formatted_name"},{"kind":"calls","target":"include"},{"kind":"calls","target":"subtitle"},{"kind":"calls","target":"track_metrics"},{"kind":"declares","target":"app/services"},{"kind":"implements","target":"BaseWorker"}]}
{"kind":"symbol","name":"Reporting::ReportWorker#bucket","file":"app/services/report_worker.rb","line":77,"repo":"ruby_sample","props":{"cyclomatic":1,"exported":false,"framework":"rails","language":"ruby","symbol_kind":"method"},"relations":[{"kind":"declares","target":"app/services"}]}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

# v97: a PRODUCTION class whose basename ends in the token `test`. Ruby's test
# globs are directory-scoped (`**/test/**/*_test.rb`), so this file — under
# app/jobs, with no `test`/`spec` directory segment — is indexed as source.
#
# Before v97 the glob was the bare suffix `**/*_test.rb`, which both ignored this
# file and routed it to reference-only test-ref extraction: the class never became
# a symbol fact and vanished from the graph. Naming a job after the A/B test it
# implements is ordinary; so are `*_load_test.rb`, `*_smoke_test.rb`.
class CacheWarmupABTest < BaseWorker
def perform(event_name, args = {})
notify(event_name, args)
end

private

def notify(event_name, args); end
end
2 changes: 1 addition & 1 deletion internal/extractors/rubyextractor/ruby_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ func collectLocals(method *sitter.Node, src []byte) map[string]bool {
// Block parameters (`things.each do |user| … end`, `{ |k, v| … }`) are locals
// too — and, being the most common identifiers inside loops, are the main
// source of false N+1 findings when their name coincides with an ActiveRecord
// association (`user`, `hood_message`, …). collectLocals is method-wide, so a
// association (`user`, `comment`, …). collectLocals is method-wide, so a
// block var here shadows a same-named bare method call elsewhere in the method;
// that over-approximation is safe (it only suppresses over-emission) and matches
// how assignment targets are already collected.
Expand Down
Loading
Loading