Skip to content

Commit 9962049

Browse files
committed
engine: count pruned directories in the receipt's skip accounting
walkRepo returned filepath.SkipDir before reaching skips.count++, so an ignored directory's files landed in no bucket: not files_seen, not files_skipped. The metric counted only ignored FILES the walker reached -- 9 on a tree with ~67k ignored files, naming none of them. A bad ignore glob, the thing this metric exists to catch, is almost always a directory glob. Count a pruned directory once, in a new dirs_skipped field, rather than walking node_modules/ purely to size it. Each skipped_sample entry now names the glob that matched it, so "why is this file missing?" is a lookup. To get the pattern, matchAnyGlob is split into matchGlob, which returns it; matchAnyGlob wraps it, so both glob lists still share one matcher. Exclude the configured output dir: .enola/** is itself a directory glob, so counting it would make dirs_skipped differ between a repo's first snapshot and every later one, and land as a phantom delta in diff_snapshot. No cacheVersion bump -- facts.jsonl is byte-identical across the fix, so no cachecov entry and no golden regeneration. Adds the tests that were missing: nothing asserted files_skipped and no golden fixture contains an ignored directory, which is how this survived 96 bumps.
1 parent c748456 commit 9962049

8 files changed

Lines changed: 292 additions & 27 deletions

File tree

ARCHITECTURE.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ After `generate_snapshot`, these are written to the output directory (default `.
574574
| `facts.jsonl` | Every extracted fact, one JSON object per line |
575575
| `insights.json` | Architectural insights with confidence scores |
576576
| `snapshot.meta.json` | Metadata including per-file content hashes for incremental updates, plus the full receipt fields |
577-
| `receipt.json` | The **snapshot receipt** — a compact manifest of what the graph was generated over (enola version, git ref + dirty status, a content-fingerprint snapshot ID, the extractor/explainer sets, ignore-glob hash, output-artifact hashes) and extraction-quality metrics (files seen/parsed/skipped, parse errors, coverage gaps). Read it via the `snapshot_receipt` tool. |
577+
| `receipt.json` | The **snapshot receipt** — a compact manifest of what the graph was generated over (enola version, git ref + dirty status, a content-fingerprint snapshot ID, the extractor/explainer sets, ignore-glob hash, output-artifact hashes) and extraction-quality metrics (files seen/parsed/skipped, directory trees pruned, parse errors, coverage gaps). Read it via the `snapshot_receipt` tool. |
578578
| `previous/` | The immediately-preceding snapshot, auto-rotated on each write — the `baseline='previous'` source for `diff_snapshot` |
579579
| `baseline/` | A snapshot pinned by `set_baseline`, preserved across re-snapshots — the default `diff_snapshot` baseline |
580580

@@ -583,7 +583,9 @@ After `generate_snapshot`, these are written to the output directory (default `.
583583
`receipt.json` (and the same fields inside `snapshot.meta.json`) exists to answer *"what was this graph deterministic over, and how complete is it?"* — the trust question before an agent relies on an `impact_analysis` or a `diff_snapshot`. It serves two consumers:
584584

585585
- **Provenance / audit.** enola version, git ref + dirty-tree status, the extractor/explainer sets actually used, a **config hash** (over the effective extractors/explainers/renderers/globs/output settings) and its narrower `ignore_glob_hash`, per-artifact output hashes, and a **snapshot ID** that is a *content fingerprint* (SHA-256 over the byte-stable fact serialization plus the version and config hash), not a random UUID — so re-running on identical inputs yields the same ID and it can key equivalence. Every hash value carries a `sha256:` prefix.
586-
- **The improvement loop.** Extraction-quality metrics — files seen vs. parsed vs. skipped, a parse-error count and sample, the count of heuristic (confidence < 1.0) insights, and the cross-repo coverage-gap / unresolved-edge rollup — give a machine-readable signal a consumer (a human, a `diff_snapshot`, or an agent improving enola itself) can poll to detect *thin extraction* (a missing detection, a bad ignore glob, a failing extractor) and turn it into targeted work. The same metrics appear as an **Extraction Quality** section in `llm_context.md`, so an agent reading the snapshot sees thin extraction without a tool call.
586+
- **The improvement loop.** Extraction-quality metrics — files seen vs. parsed vs. skipped, the number of directory trees pruned, a parse-error count and sample, the count of heuristic (confidence < 1.0) insights, and the cross-repo coverage-gap / unresolved-edge rollup — give a machine-readable signal a consumer (a human, a `diff_snapshot`, or an agent improving enola itself) can poll to detect *thin extraction* (a missing detection, a bad ignore glob, a failing extractor) and turn it into targeted work. The same metrics appear as an **Extraction Quality** section in `llm_context.md`, so an agent reading the snapshot sees thin extraction without a tool call.
587+
588+
The two skip counters mean different things, and a bad ignore glob is usually a *directory* glob. `files_skipped` counts ignored files the walker **visited** and dropped — those matched by a file glob like `**/*.test.ts`. An ignored directory is pruned whole (`filepath.SkipDir`), so its contents are never visited and appear in no count: it is tallied once, as one entry in `dirs_skipped`. Each `skipped_sample` entry names the glob that matched it, so *"why is this file missing from the graph?"* is a lookup rather than an investigation; directories appear there with a trailing slash.
587589

588590
Because the receipt fields live in `snapshot.meta.json`, they ride into every pinned/`previous` baseline, and `diff_snapshot` reads them to add a **comparability guard**: it warns (above the delta) when the baseline and current snapshots were *not* generated over equivalent inputs — a different repo, enola version, extractor set, or ignore-glob set — since a diff across a mismatched extractor set would report every one of that language's facts as spurious churn. `compare_receipts` surfaces the same verdict plus the metric deltas directly.
589591

internal/diff/receipt.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ func CompareReceipts(base, cur facts.SnapshotMeta) *ReceiptComparison {
5454
delta("files_seen", base.FilesSeen, cur.FilesSeen),
5555
delta("files_parsed", base.FilesParsed, cur.FilesParsed),
5656
delta("files_skipped", base.FilesSkipped, cur.FilesSkipped),
57+
delta("dirs_skipped", base.DirsSkipped, cur.DirsSkipped),
5758
delta("parse_errors", base.ParseErrors, cur.ParseErrors),
5859
delta("coverage_gaps", baseGaps, curGaps),
5960
delta("unresolved_edges", baseUnresolved, curUnresolved),

internal/diff/receipt_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,31 @@ func TestCompareReceipts(t *testing.T) {
114114
t.Error("expected Identical=true for matching snapshot_id")
115115
}
116116
})
117+
118+
// A newly-ignored directory prunes a whole subtree. It reports as a dirs_skipped
119+
// delta and NOT as a quality regression: pruning vendor/ is usually the operator
120+
// doing the right thing, and files_seen falling is the signal that costs something.
121+
t.Run("a pruned directory surfaces as a dirs_skipped delta", func(t *testing.T) {
122+
cur := base
123+
cur.SnapshotID = "bbb"
124+
cur.DirsSkipped = 2
125+
126+
rc := CompareReceipts(base, cur)
127+
128+
var found *MetricDelta
129+
for i := range rc.Deltas {
130+
if rc.Deltas[i].Name == "dirs_skipped" {
131+
found = &rc.Deltas[i]
132+
}
133+
}
134+
if found == nil {
135+
t.Fatalf("no dirs_skipped delta; got %v", rc.Deltas)
136+
}
137+
if found.Before != 0 || found.After != 2 || found.Delta != 2 {
138+
t.Errorf("dirs_skipped delta = %+v, want before=0 after=2 delta=2", *found)
139+
}
140+
if len(rc.QualityRegressions) != 0 {
141+
t.Errorf("pruning a directory is not a regression, got: %v", rc.QualityRegressions)
142+
}
143+
})
117144
}

internal/engine/engine.go

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ func (e *Engine) GenerateSnapshot(ctx context.Context, repoPath string, appendMo
317317
FilesSeen: len(files),
318318
FilesParsed: e.store.CountFilesWithFacts(files, parsedPrefix),
319319
FilesSkipped: skips.count,
320+
DirsSkipped: skips.dirCount,
320321
SkippedSample: skips.sample,
321322
IgnoreGlobHash: ignoreGlobHash,
322323
ParseErrors: len(parseErrs),
@@ -442,12 +443,29 @@ func (e *Engine) flagUnmatchedRoutes() {
442443
}
443444
}
444445

445-
// walkSkips is a lightweight tally of files dropped by the ignore globs, kept so
446-
// a snapshot receipt can report how much of the tree was excluded (and a sample
447-
// of what) without retaining every skipped path.
446+
// walkSkips is a lightweight tally of what the ignore globs dropped, kept so a
447+
// snapshot receipt can report how much of the tree was excluded (and a sample of
448+
// what) without retaining every skipped path.
449+
//
450+
// Files and directories are tallied separately because they cost differently to
451+
// know. An ignored directory is pruned whole — the walker never descends, so its
452+
// contents are counted nowhere. Counting them would mean walking node_modules/
453+
// purely to size it, a stat per file for a number no architecture graph wants.
454+
// One pruned directory is one architecturally meaningful fact; its 55,041 files
455+
// are not.
448456
type walkSkips struct {
449-
count int
450-
sample []string
457+
count int // ignored FILES the walker visited
458+
dirCount int // ignored DIRECTORIES pruned; their contents are never visited
459+
sample []string // capped sample of both, each annotated with the glob that matched
460+
}
461+
462+
// record appends "<path> (glob: <pattern>)" until the sample is full. Directories
463+
// arrive with a trailing slash, which is what distinguishes a pruned subtree from
464+
// a single dropped file in the receipt.
465+
func (s *walkSkips) record(path, pattern string) {
466+
if len(s.sample) < skippedSampleCap {
467+
s.sample = append(s.sample, path+" (glob: "+pattern+")")
468+
}
451469
}
452470

453471
// skippedSampleCap bounds the number of skipped paths retained for the receipt.
@@ -456,8 +474,8 @@ const skippedSampleCap = 20
456474
// walkRepo collects all files in the repo, applying ignore patterns. It returns
457475
// the indexable source files, separately the test/spec files matched by
458476
// TestGlobs (excluded from normal indexing but collected for reference-only
459-
// extraction — see runTestRefExtractors), and a tally of ignored files for the
460-
// snapshot receipt.
477+
// extraction — see runTestRefExtractors), and a tally of what the ignore globs
478+
// dropped — ignored files, and pruned directories — for the snapshot receipt.
461479
func (e *Engine) walkRepo(repoPath string) (files, testFiles []string, skips walkSkips, err error) {
462480
err = filepath.WalkDir(repoPath, func(path string, d fs.DirEntry, err error) error {
463481
if err != nil {
@@ -470,8 +488,15 @@ func (e *Engine) walkRepo(repoPath string) (files, testFiles []string, skips wal
470488
}
471489

472490
// Skip ignored paths
473-
if e.isIgnored(relPath, d.IsDir()) {
491+
if pattern, ok := e.ignoreMatch(relPath); ok {
474492
if d.IsDir() {
493+
// enola's own output directory is not part of the source tree.
494+
// Counting it would make dirs_skipped differ between a repo's
495+
// first-ever snapshot and every one after it, for no signal.
496+
if relPath != e.cfg.Output.Dir {
497+
skips.dirCount++
498+
skips.record(filepath.ToSlash(relPath)+"/", pattern)
499+
}
475500
return filepath.SkipDir
476501
}
477502
// An ignored FILE that is a test/spec is not indexed as production
@@ -481,9 +506,7 @@ func (e *Engine) walkRepo(repoPath string) (files, testFiles []string, skips wal
481506
testFiles = append(testFiles, relPath)
482507
}
483508
skips.count++
484-
if len(skips.sample) < skippedSampleCap {
485-
skips.sample = append(skips.sample, filepath.ToSlash(relPath))
486-
}
509+
skips.record(filepath.ToSlash(relPath), pattern)
487510
return nil
488511
}
489512

@@ -503,7 +526,15 @@ func (e *Engine) matchesTestGlob(relPath string) bool {
503526
// matchAnyGlob reports whether a forward-slash path matches any of the patterns.
504527
// It is the single matcher behind both the ignore list and the test globs, so a
505528
// file the two lists disagree about cannot exist: an ignored file that stops being
506-
// a test necessarily stops being ignored. Supported forms:
529+
// a test necessarily stops being ignored.
530+
func matchAnyGlob(relPath string, patterns []string) bool {
531+
_, ok := matchGlob(relPath, patterns)
532+
return ok
533+
}
534+
535+
// matchGlob returns the first pattern that matches relPath. The receipt records it
536+
// beside the skipped path, so "why is this file missing from the graph?" is a
537+
// lookup rather than an investigation. Supported forms:
507538
//
508539
// vendor/** anchored directory prefix
509540
// **/build/** a directory named "build" at any depth
@@ -512,7 +543,7 @@ func (e *Engine) matchesTestGlob(relPath string) bool {
512543
//
513544
// The last form is the only one that constrains directory and filename together;
514545
// see matchDirScopedGlob for why the Ruby test globs need it.
515-
func matchAnyGlob(relPath string, patterns []string) bool {
546+
func matchGlob(relPath string, patterns []string) (string, bool) {
516547
for _, pattern := range patterns {
517548
// "<prefix>/**/<fileglob>". Handled first and exclusively: the branches
518549
// below would match such a pattern only when exactly one directory sits
@@ -522,7 +553,7 @@ func matchAnyGlob(relPath string, patterns []string) bool {
522553
prefix, fileGlob := pattern[:i], pattern[i+len("/**/"):]
523554
if !strings.Contains(fileGlob, "/") {
524555
if matchDirScopedGlob(relPath, prefix, fileGlob) {
525-
return true
556+
return pattern, true
526557
}
527558
continue
528559
}
@@ -532,31 +563,31 @@ func matchAnyGlob(relPath string, patterns []string) bool {
532563
if seg != "" && !strings.Contains(seg, "/") {
533564
for _, part := range strings.Split(relPath, "/") {
534565
if part == seg {
535-
return true
566+
return pattern, true
536567
}
537568
}
538569
}
539570
}
540571
if strings.HasSuffix(pattern, "/**") {
541572
dirPrefix := strings.TrimSuffix(pattern, "/**")
542573
if relPath == dirPrefix || strings.HasPrefix(relPath, dirPrefix+"/") {
543-
return true
574+
return pattern, true
544575
}
545576
}
546577
if m, err := filepath.Match(pattern, relPath); err == nil && m {
547-
return true
578+
return pattern, true
548579
}
549580
if strings.HasPrefix(pattern, "**/") {
550581
sub := strings.TrimPrefix(pattern, "**/")
551582
if m, err := filepath.Match(sub, filepath.Base(relPath)); err == nil && m {
552-
return true
583+
return pattern, true
553584
}
554585
if m, err := filepath.Match(sub, relPath); err == nil && m {
555-
return true
586+
return pattern, true
556587
}
557588
}
558589
}
559-
return false
590+
return "", false
560591
}
561592

562593
// matchDirScopedGlob reports whether relPath's basename matches fileGlob AND
@@ -602,6 +633,12 @@ func (e *Engine) isIgnored(relPath string, isDir bool) bool {
602633
return matchAnyGlob(filepath.ToSlash(relPath), e.cfg.Ignore)
603634
}
604635

636+
// ignoreMatch reports whether a path is ignored, and by which pattern. The walker
637+
// needs the pattern to record it in the receipt's skipped sample.
638+
func (e *Engine) ignoreMatch(relPath string) (string, bool) {
639+
return matchGlob(filepath.ToSlash(relPath), e.cfg.Ignore)
640+
}
641+
605642
// runExtractors detects applicable extractors and runs them. When cache is
606643
// non-nil, extractors implementing plugin.FileOwner have their facts reused
607644
// whenever the files they depend on are unchanged since the last snapshot.

0 commit comments

Comments
 (0)