From 7be5a69c87be8a54946ffb6d82a9a59aec0e099f Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Wed, 5 Aug 2026 03:52:09 +0300 Subject: [PATCH] fix(optimize): one junk vocabulary for the count, the trend and the display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit detectJunkReads took its count from core's detector and re-derived its display and trend from the host's own JUNK_DIRS regex. The two lists disagreed: core classifies `vendor`, `site-packages`, `out` and `target` as junk; the host's did not. Core was already counting reads under those segments — the count is not what was wrong. The host's derivation was. In a repo whose junk reads all live under one of them — Go and PHP vendor, Python site-packages, Rust target, Java or Next out — the host loop matched nothing, so `recentJunkReads` stayed at zero. Where the window also had recent activity, `computeTrend` read that as fixed and returned 'resolved', and the finding was dropped. Whole ecosystems never saw it. In mixed repos it survived but rendered incoherently: the explanation quoted core's total while the directory list and the CLAUDE.md suggestion came from the narrower host counts, so the numbers did not add up and the suggested directories omitted the one actually causing the waste. Core now exports `junkSegmentOf`, which returns the exact segment that made a path junk. The host deletes JUNK_DIRS and JUNK_PATTERN outright and asks core in both loops — junk-reads and duplicate-reads, which had the same split. The host still names the directory for the payload, because a class alone cannot render `vendor/ (5x)`; it just no longer decides what junk means. Inside core, the precedence rule (dependency > build > vcs) lived in the order of three loops, and adding junkSegmentOf duplicated them. Both functions now consult one private helper, so the two cannot drift — a second copy of one rule is what caused this bug in the first place. Two display changes fall out. A path under two junk segments of the same table now names the first in path order rather than the first in array order, so `/x/build/dist/y` reports `build` where it used to report `dist`; the count is unchanged. Terminal junk directories and Windows-style paths are now matched, which the old slash-delimited regex missed — those align the host with what core was already counting. One robustness note: the old `JUNK_PATTERN.test()` coerced a truthy non-string `file_path`, while `junkSegmentOf` would throw on one. The type is narrowed at the point of use so the new path cannot throw where the old one could not. --- CHANGELOG.md | 1 + packages/cli/src/optimize.ts | 44 +++++++++++--------- packages/cli/tests/optimize.test.ts | 40 +++++++++++++++++- packages/core/src/fingerprint.ts | 54 ++++++++++++++++++++----- packages/core/tests/fingerprint.test.ts | 29 +++++++++++++ 5 files changed, 139 insertions(+), 29 deletions(-) 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'],