diff --git a/CHANGELOG.md b/CHANGELOG.md index 98c38076..36feb4a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixed - Claude Desktop and Cowork sessions are discovered for Windows Microsoft Store (MSIX) installs. (#611) +- The build-folder-reads finding is no longer dropped for repos whose junk reads live only under `vendor/`, `site-packages/`, `out/` or `target/` (Go, PHP, Python, Rust, and Java/Next). Core's detector already counted those reads — only the host's display and trend derivation used a narrower directory list, so when the window had recent activity the recent count stayed zero, the trend computed as 'resolved', and the finding vanished entirely. In mixed repos it survived but quoted core's total while listing directories from the host's narrower counts, so the numbers disagreed. Junk-read and duplicate-read detection now ask core for the offending segment, so the count, the directory list and the trend share one vocabulary; the count itself is unchanged. ## 0.9.19 - 2026-07-20 diff --git a/packages/cli/src/optimize.ts b/packages/cli/src/optimize.ts index 053ad0da..caf510da 100644 --- a/packages/cli/src/optimize.ts +++ b/packages/cli/src/optimize.ts @@ -4,7 +4,7 @@ import { existsSync, statSync } from 'fs' import { basename, join } from 'path' import { homedir } from 'os' -import { projectRef as fingerprintProjectRef, resourceFingerprint, sessionRef as fingerprintSessionRef } from '@codeburn/core/fingerprint' +import { junkSegmentOf, projectRef as fingerprintProjectRef, resourceFingerprint, sessionRef as fingerprintSessionRef } from '@codeburn/core/fingerprint' import { OBSERVATION_SCHEMA_VERSION } from '@codeburn/core/schema' import type { CallObservation, ObservationEnvelope, SessionObservation } from '@codeburn/core/observations' import type { Finding } from '@codeburn/core/contracts' @@ -209,12 +209,10 @@ const MAX_IMPORT_DEPTH = 5 const IMPORT_PATTERN = /^@(\.\.?\/[^\s]+|\/[^\s]+)/gm const COMMAND_PATTERN = /([^<]+)<\/command-name>|(?:^|\s)\/([a-zA-Z][\w-]*)/gm -const JUNK_DIRS = [ - 'node_modules', '.git', 'dist', 'build', '__pycache__', '.next', - '.nuxt', '.output', 'coverage', '.cache', '.tsbuildinfo', - '.venv', 'venv', '.svn', '.hg', -] -const JUNK_PATTERN = new RegExp(`/(?:${JUNK_DIRS.join('|')})/`) +// What counts as a junk path (dependency/build/vcs) is core's decision alone — +// junkSegmentOf (@codeburn/core/fingerprint) classifies against the same segment +// tables the junk-reads/duplicate-reads detectors use. The host never re-tests +// paths against its own list; it only names the offending segment for display. const SHELL_PROFILES = ['.zshrc', '.bashrc', '.bash_profile', '.profile'] @@ -757,20 +755,25 @@ export function detectJunkReads(calls: ToolCall[], dateRange?: DateRange): Waste const totalJunkReads = evidenceCount(findings[0], 'junk-reads') const tokensSaved = evidenceCount(findings[0], 'tokens-saved') - // Display + trend stay host-derived from the raw path data (D5-A). + // The junk DECISION comes from core's classification — one vocabulary with + // the detector that produced the count, so recentJunkReads can never disagree + // with totalJunkReads (that mismatch used to 'resolve' the finding away for + // vendor/ site-packages/ out/ target/ repos). The host still names the + // offending directory from the raw path it retained (D5-A): a class alone + // cannot render 'vendor/ (7x)'. const dirCounts = new Map() let recentJunkReads = 0 for (const call of calls) { if (!isReadTool(call.name)) continue - const filePath = call.input.file_path as string | undefined - if (!filePath || !JUNK_PATTERN.test(filePath)) continue + const filePath = call.input.file_path + // JUNK_PATTERN.test() coerced a truthy non-string instead of throwing; + // junkSegmentOf -> normalizePath would throw on one, so narrow the type + // here rather than letting a non-string reach core's classifier. + if (typeof filePath !== 'string') continue + const seg = junkSegmentOf(filePath) + if (!seg) continue if (call.recent) recentJunkReads++ - for (const dir of JUNK_DIRS) { - if (filePath.includes(`/${dir}/`)) { - dirCounts.set(dir, (dirCounts.get(dir) ?? 0) + 1) - break - } - } + dirCounts.set(seg, (dirCounts.get(seg) ?? 0) + 1) } const hasRecentActivity = calls.some(c => c.recent) @@ -808,11 +811,16 @@ export function detectDuplicateReads(calls: ToolCall[], dateRange?: DateRange): const tokensSaved = evidenceCount(findings[0], 'tokens-saved') // Per-file breakdown + trend stay host-derived from the raw path data (D5-A). + // Junk exclusion asks core (junkSegmentOf) so the host drops exactly the + // resources the duplicate-reads detector excluded — the legacy narrow regex + // let vendor/ etc. through, putting non-counted files in the display. const sessionFiles = new Map>() for (const call of calls) { if (!isReadTool(call.name)) continue - const filePath = call.input.file_path as string | undefined - if (!filePath || JUNK_PATTERN.test(filePath)) continue + const filePath = call.input.file_path + // Same type guard as detectJunkReads: junkSegmentOf -> normalizePath throws + // on a truthy non-string where JUNK_PATTERN.test() used to coerce it. + if (typeof filePath !== 'string' || junkSegmentOf(filePath)) continue const key = `${call.project}:${call.sessionId}` if (!sessionFiles.has(key)) sessionFiles.set(key, new Map()) const fm = sessionFiles.get(key)! diff --git a/packages/cli/tests/optimize.test.ts b/packages/cli/tests/optimize.test.ts index fdbd4926..36d99c92 100644 --- a/packages/cli/tests/optimize.test.ts +++ b/packages/cli/tests/optimize.test.ts @@ -174,12 +174,16 @@ describe('detectJunkReads', () => { expect(detectJunkReads(calls)).toBeNull() }) - it('handles missing file_path gracefully', () => { + it('handles missing or non-string file_path gracefully', () => { const calls = [ call('Read', {}), call('Read', { file_path: null as unknown as string }), + // A truthy non-string must not throw: JUNK_PATTERN.test() coerced it, + // junkSegmentOf -> normalizePath would not. + call('Read', { file_path: 42 as unknown as string }), ] expect(detectJunkReads(calls)).toBeNull() + expect(() => detectDuplicateReads(calls)).not.toThrow() }) it('suggests CLAUDE.md advice listing detected and common junk dirs', () => { @@ -195,6 +199,26 @@ describe('detectJunkReads', () => { } expect(finding.fix.label).toContain('CLAUDE.md') }) + + it('flags a vendor-only project even with recent activity (was silently resolved)', () => { + // Regression: core classifies vendor/ as junk, but the host's legacy + // JUNK_DIRS regex did not, so recentJunkReads stayed 0 and computeTrend + // returned 'resolved' — the whole finding vanished for Go/PHP repos. + const calls = Array.from({ length: 5 }, (_, i) => ({ + ...call('Read', { file_path: `/go/src/app/vendor/lib-${i}.go` }), + recent: true, + })) + const finding = detectJunkReads(calls) + expect(finding).not.toBeNull() + expect(finding!.trend).not.toBe('resolved') + // Display names the segment from core's vocabulary, and the explanation's + // count is core's total — the two no longer disagree. + expect(finding!.explanation).toContain('vendor/ (5x)') + expect(finding!.explanation).toContain('(5 reads)') + if (finding!.fix.type === 'paste') { + expect(finding!.fix.text).toContain('vendor') + } + }) }) describe('detectDuplicateReads', () => { @@ -223,6 +247,20 @@ describe('detectDuplicateReads', () => { expect(detectDuplicateReads(calls)).toBeNull() }) + it('excludes vendor reads from the duplicate display (core vocabulary)', () => { + // core's duplicate-reads detector excludes every junk class (vendor + // included); the host-side per-file display must drop the same paths, + // or it names files that never contributed to the count. + const calls = [ + ...Array.from({ length: 7 }, () => call('Read', { file_path: '/src/a.ts' }, 's1')), + ...Array.from({ length: 7 }, () => call('Read', { file_path: '/go/app/vendor/dep.ts' }, 's1')), + ] + const finding = detectDuplicateReads(calls) + expect(finding).not.toBeNull() + expect(finding!.explanation).toContain('a.ts') + expect(JSON.stringify(finding)).not.toContain('dep.ts') + }) + it('returns null for single reads', () => { const calls = [ call('Read', { file_path: '/src/a.ts' }, 's1'), diff --git a/packages/core/src/fingerprint.ts b/packages/core/src/fingerprint.ts index b0343fa5..9b2947dd 100644 --- a/packages/core/src/fingerprint.ts +++ b/packages/core/src/fingerprint.ts @@ -45,7 +45,9 @@ export interface ResourceFingerprint { // CLI's legacy JUNK_DIRS regex: every directory that regex named classifies here // as junk too (the extras — 'venv', '__pycache__', 'coverage', '.cache', // '.nuxt', '.output', '.svn', '.hg' — are added below), plus vendor / -// site-packages / out / target, which the old regex missed. +// site-packages / out / target, which the old regex missed. The host CLI must +// consume the tables through junkSegmentOf (below) rather than re-testing paths +// against its own regex, so the junk decision is one vocabulary everywhere. const DEPENDENCY_SEGMENTS = new Set(['node_modules', 'vendor', '.venv', 'venv', 'site-packages']) const BUILD_SEGMENTS = new Set([ 'dist', 'build', 'out', 'target', '.next', '.nuxt', '.output', @@ -85,6 +87,27 @@ function extensionOf(basename: string): string | undefined { return basename.slice(dot + 1).toLowerCase() } +/** + * First path segment that makes the path junk, in precedence order + * (dependency > build > vcs), along with the class it implies; or null. + * This is the single home of the junk precedence rule — classifyResource and + * junkSegmentOf both consult it, so the two cannot drift apart. + */ +function firstJunk( + segments: string[], +): { segment: string; resourceClass: 'dependency' | 'build' | 'vcs' } | null { + for (const seg of segments) { + if (DEPENDENCY_SEGMENTS.has(seg)) return { segment: seg, resourceClass: 'dependency' } + } + for (const seg of segments) { + if (BUILD_SEGMENTS.has(seg)) return { segment: seg, resourceClass: 'build' } + } + for (const seg of segments) { + if (VCS_SEGMENTS.has(seg)) return { segment: seg, resourceClass: 'vcs' } + } + return null +} + /** * Classify a path by its segments and basename. Precedence is directory-based * first (a file under node_modules is a dependency regardless of its @@ -95,15 +118,8 @@ export function classifyResource(absolutePath: string): ResourceClass { const normalized = normalizePath(absolutePath) const segments = normalized.split('/').filter(Boolean) - for (const seg of segments) { - if (DEPENDENCY_SEGMENTS.has(seg)) return 'dependency' - } - for (const seg of segments) { - if (BUILD_SEGMENTS.has(seg)) return 'build' - } - for (const seg of segments) { - if (VCS_SEGMENTS.has(seg)) return 'vcs' - } + const junk = firstJunk(segments) + if (junk) return junk.resourceClass const basename = segments[segments.length - 1] ?? '' // A dotfile (e.g. `.eslintrc`, `.gitignore`) is configuration. @@ -118,6 +134,24 @@ export function classifyResource(absolutePath: string): ResourceClass { return 'other' } +/** + * If `absolutePath` classifies as junk (resourceClass ∈ dependency/build/vcs, + * see JUNK_RESOURCE_CLASSES), return the exact path segment that made it junk + * ('node_modules', 'vendor', 'out', ...); otherwise null. Precedence matches + * classifyResource (dependency > build > vcs), so the returned segment is the + * one that determined the class. + * + * The host CLI uses this to keep its display and its junk decision on core's + * vocabulary: it still names the offending directory for the payload — a class + * alone cannot render 'node_modules/ (7x)' — but never re-tests paths against + * its own copy of the tables. + */ +export function junkSegmentOf(absolutePath: string): string | null { + const normalized = normalizePath(absolutePath) + const segments = normalized.split('/').filter(Boolean) + return firstJunk(segments)?.segment ?? null +} + /** * Fingerprint an absolute path into `{ resourceClass, resourceId }`. The class * is a coarse, non-identifying bucket; the id is the domain-separated HMAC of diff --git a/packages/core/tests/fingerprint.test.ts b/packages/core/tests/fingerprint.test.ts index 2b71a206..e24b344f 100644 --- a/packages/core/tests/fingerprint.test.ts +++ b/packages/core/tests/fingerprint.test.ts @@ -4,6 +4,7 @@ import { branchRef, classifyResource, commandFamily, + junkSegmentOf, normalizePath, projectRef, resourceFingerprint, @@ -130,6 +131,34 @@ describe('resource classification', () => { }) }) +describe('junkSegmentOf', () => { + it('returns the exact segment that made the path junk', () => { + expect(junkSegmentOf('/repo/node_modules/lodash/index.js')).toBe('node_modules') + expect(junkSegmentOf('/go/src/app/vendor/lib.go')).toBe('vendor') + expect(junkSegmentOf('/py/.venv/lib/site.py')).toBe('.venv') + expect(junkSegmentOf('/py/venv/lib/site.py')).toBe('venv') + expect(junkSegmentOf('/py/site-packages/x.py')).toBe('site-packages') + expect(junkSegmentOf('/repo/dist/index.js')).toBe('dist') + expect(junkSegmentOf('/rs/target/debug/app')).toBe('target') + expect(junkSegmentOf('/repo/.next/server/page.js')).toBe('.next') + expect(junkSegmentOf('/repo/.git/HEAD')).toBe('.git') + }) + + it('follows classifyResource precedence (dependency > build > vcs)', () => { + // The segment returned must be the one that determined the class. + expect(junkSegmentOf('/a/vendor/b/out/c')).toBe('vendor') + expect(junkSegmentOf('/a/out/b/.git/c')).toBe('out') + }) + + it('returns null for non-junk paths, including .tsbuildinfo files', () => { + expect(junkSegmentOf('/repo/src/a.ts')).toBeNull() + expect(junkSegmentOf('/repo/README.md')).toBeNull() + // '.tsbuildinfo' names a file (tsconfig.tsbuildinfo), never a directory + // segment, so it is deliberately not junk (see junk-reads detector notes). + expect(junkSegmentOf('/repo/foo.tsbuildinfo')).toBeNull() + }) +}) + describe('commandFamily (leading token only)', () => { const cases: Array<[string, string]> = [ ['git commit -m "x"', 'git'],