diff --git a/CHANGELOG.md b/CHANGELOG.md index 94c58dd..34e1305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,10 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Matcher cache reset and stats helpers for deterministic cache verification. + +### Changed + +- Glob matching now reuses a bounded compiled matcher cache instead of recompiling picomatch patterns for every file. +- Dynamic rule loading now deduplicates repeated target paths and rule-file parsing work. + ### Fixed - Dynamic rule injection now dedupes by rule across the session instead of per tool call, preventing repeated nested `AGENTS.md`/`CLAUDE.md` instruction blocks on subsequent reads. - Dynamic injection now skips rules already injected statically or already loaded by pi's native context loader. +- Dynamic rule loading now preserves each target file's project root so nested projects load their nearest rules correctly. ## [0.1.0] - 2026-04-29 diff --git a/src/index.ts b/src/index.ts index 67b6540..6e3b279 100644 --- a/src/index.ts +++ b/src/index.ts @@ -116,14 +116,16 @@ export default function piRulesExtension(pi: ExtensionAPI): void { } const loaded = engine.loadDynamicRules(ctx.cwd, targetPaths); - const rules = loaded.rules.filter((rule) => !engine.isStaticInjected(rule) && !engine.isDynamicInjected(rule)); + const rules = loaded.rules.filter( + (rule) => !engine.isStaticInjected(rule) && !engine.isDynamicInjected(firstTargetPath, rule), + ); if (rules.length === 0) { return undefined; } const block = engine.formatDynamic(rules, displayPath(ctx.cwd, firstTargetPath)); for (const rule of rules) { - engine.markDynamicInjected(rule); + engine.markDynamicInjected(firstTargetPath, rule); } return { content: [...event.content, { type: "text", text: block }] }; diff --git a/src/rules/cache.ts b/src/rules/cache.ts index ab8153a..ec7732c 100644 --- a/src/rules/cache.ts +++ b/src/rules/cache.ts @@ -1,7 +1,5 @@ import type { LoadedRule, SessionState } from "./types.js"; -const DYNAMIC_SESSION_KEY = "__pi-rules-session__"; - export function createSessionState(cwd?: string): SessionState { return { cwd, staticDedup: new Set(), dynamicDedup: new Map(), loadedRules: [], diagnostics: [] }; } @@ -10,8 +8,8 @@ export function staticDedupKey(cwd: string, rulePath: string, contentHash: strin return `${cwd}::${rulePath}::${contentHash}`; } -export function dynamicDedupKey(rulePath: string, contentHash: string): string { - return `${rulePath}::${contentHash}`; +export function dynamicDedupKey(scopeKey: string, rulePath: string, contentHash: string): string { + return `${scopeKey}::${rulePath}::${contentHash}`; } export function markStaticInjected(state: SessionState, rule: LoadedRule): boolean { @@ -24,14 +22,14 @@ export function markStaticInjected(state: SessionState, rule: LoadedRule): boole return true; } -export function markDynamicInjected(state: SessionState, rule: LoadedRule): boolean { - let keys = state.dynamicDedup.get(DYNAMIC_SESSION_KEY); +export function markDynamicInjected(state: SessionState, scopeKey: string, rule: LoadedRule): boolean { + let keys = state.dynamicDedup.get(scopeKey); if (keys === undefined) { keys = new Set(); - state.dynamicDedup.set(DYNAMIC_SESSION_KEY, keys); + state.dynamicDedup.set(scopeKey, keys); } - const key = dynamicDedupKey(rule.realPath, rule.contentHash); + const key = dynamicDedupKey(scopeKey, rule.realPath, rule.contentHash); if (keys.has(key)) { return false; } @@ -44,8 +42,8 @@ export function isStaticInjected(state: SessionState, rule: LoadedRule): boolean return state.staticDedup.has(staticDedupKey(state.cwd ?? "", rule.realPath, rule.contentHash)); } -export function isDynamicInjected(state: SessionState, rule: LoadedRule): boolean { - return state.dynamicDedup.get(DYNAMIC_SESSION_KEY)?.has(dynamicDedupKey(rule.realPath, rule.contentHash)) === true; +export function isDynamicInjected(state: SessionState, scopeKey: string, rule: LoadedRule): boolean { + return state.dynamicDedup.get(scopeKey)?.has(dynamicDedupKey(scopeKey, rule.realPath, rule.contentHash)) === true; } export function clearSession(state: SessionState): void { diff --git a/src/rules/engine.ts b/src/rules/engine.ts index c5c5f89..aff45bc 100644 --- a/src/rules/engine.ts +++ b/src/rules/engine.ts @@ -22,6 +22,18 @@ import { sortCandidates } from "./ordering.js"; import { parseRule } from "./parser.js"; import type { LoadedRule, MatchReason, PiRulesConfig, RuleCandidate, RuleDiagnostic, SessionState } from "./types.js"; +interface LoadedRuleContent { + frontmatter: LoadedRule["frontmatter"]; + body: string; + contentHash: string; + diagnostic?: string; +} + +type CandidateProjectMembership = Map; +type DynamicMatchCache = Map; + +const MAX_DYNAMIC_MATCH_CACHE_ENTRIES = 4096; + export interface EngineDeps { findCandidates: (options: { projectRoot: string | null; @@ -33,6 +45,7 @@ export interface EngineDeps { readFile: (path: string) => string | null; findProjectRoot: (startPath: string) => string | null; extractToolPaths: (event: ToolResultEvent, cwd: string) => string[]; + matchRule?: typeof matchRule; } export interface Engine { @@ -47,9 +60,9 @@ export interface Engine { formatDynamic(rules: ReadonlyArray, target: string): string; resetSession(cwd?: string): void; isStaticInjected(rule: LoadedRule): boolean; - isDynamicInjected(rule: LoadedRule): boolean; + isDynamicInjected(scopeKey: string, rule: LoadedRule): boolean; markStaticInjected(rule: LoadedRule): boolean; - markDynamicInjected(rule: LoadedRule): boolean; + markDynamicInjected(scopeKey: string, rule: LoadedRule): boolean; } const ROOT_SINGLE_FILE_SOURCES = new Set(PROJECT_SINGLE_FILES.filter((source) => !source.includes("/"))); @@ -66,6 +79,7 @@ export function defaultConfig(): PiRulesConfig { export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const state = createSessionState(); + const dynamicMatchCache: DynamicMatchCache = new Map(); function loadStaticRules(cwd: string): { rules: LoadedRule[]; diagnostics: RuleDiagnostic[] } { state.cwd = cwd; @@ -96,25 +110,37 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { const rules: LoadedRule[] = []; const diagnostics: RuleDiagnostic[] = []; const seenRules = new Set(); + const loadedRuleContent = new Map(); + const projectMembership = new Map(); const disabledSources = disabledSourcesFor(config); - for (const targetFile of targetPaths) { + for (const targetFile of uniqueStrings(targetPaths)) { const projectRoot = deps.findProjectRoot(targetFile); const candidates = deps.findCandidates({ projectRoot, targetFile, disabledSources }); for (const candidate of sortCandidates(candidates)) { - const loadedRule = loadCandidate(candidate, deps, diagnostics, projectRoot); + const loadedRule = loadCandidate( + candidate, + deps, + diagnostics, + projectRoot, + loadedRuleContent, + projectMembership, + ); if (loadedRule === null) { continue; } - const matchResult = matchRule({ - frontmatter: loadedRule.frontmatter, - isSingleFile: candidate.isSingleFile, - pathBases: pathBasesForTarget(projectRoot, targetFile, candidate), - }); + const matchReason = matchDynamicRuleCached( + dynamicMatchCache, + projectRoot, + targetFile, + candidate, + loadedRule, + deps.matchRule ?? matchRule, + ); - if (!matchResult.matched) { + if (matchReason === null) { continue; } @@ -124,7 +150,7 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { } seenRules.add(dedupKey); - rules.push({ ...loadedRule, matchReason: matchResult.reason }); + rules.push({ ...loadedRule, matchReason }); } } @@ -147,17 +173,73 @@ export function createEngine(config: PiRulesConfig, deps: EngineDeps): Engine { }), resetSession: (cwd) => { clearSession(state); + dynamicMatchCache.clear(); if (cwd !== undefined) { state.cwd = cwd; } }, isStaticInjected: (rule) => isStaticInjectedInState(state, rule), - isDynamicInjected: (rule) => isDynamicInjectedInState(state, rule), + isDynamicInjected: (scopeKey, rule) => isDynamicInjectedInState(state, scopeKey, rule), markStaticInjected: (rule) => markStaticInjectedInState(state, rule), - markDynamicInjected: (rule) => markDynamicInjectedInState(state, rule), + markDynamicInjected: (scopeKey, rule) => markDynamicInjectedInState(state, scopeKey, rule), }; } +function matchDynamicRuleCached( + cache: DynamicMatchCache, + projectRoot: string | null, + targetFile: string, + candidate: RuleCandidate, + loadedRule: LoadedRule, + matchRuleImpl: typeof matchRule, +): MatchReason | null { + const cacheKey = dynamicMatchCacheKey(projectRoot, targetFile, candidate, loadedRule.contentHash); + if (cache.has(cacheKey)) { + const cachedReason = cache.get(cacheKey) ?? null; + cache.delete(cacheKey); + cache.set(cacheKey, cachedReason); + return cachedReason; + } + + const matchResult = matchRuleImpl({ + frontmatter: loadedRule.frontmatter, + isSingleFile: candidate.isSingleFile, + pathBases: pathBasesForTarget(projectRoot, targetFile, candidate), + }); + const reason = matchResult.matched ? matchResult.reason : null; + setDynamicMatchCacheEntry(cache, cacheKey, reason); + return reason; +} + +function setDynamicMatchCacheEntry(cache: DynamicMatchCache, cacheKey: string, reason: MatchReason | null): void { + if (cache.size >= MAX_DYNAMIC_MATCH_CACHE_ENTRIES) { + const oldestCacheKey = cache.keys().next().value; + if (oldestCacheKey !== undefined) { + cache.delete(oldestCacheKey); + } + } + cache.set(cacheKey, reason); +} + +function dynamicMatchCacheKey( + projectRoot: string | null, + targetFile: string, + candidate: RuleCandidate, + contentHash: string, +): string { + return [ + projectRoot ?? "", + toPosixPath(resolve(targetFile)), + candidate.realPath, + candidate.relativePath, + candidate.source, + candidate.isGlobal ? "global" : "project", + candidate.isSingleFile ? "single" : "multi", + String(candidate.distance), + contentHash, + ].join("\0"); +} + function loadStaticCandidates(candidates: ReadonlyArray, deps: EngineDeps, projectRoot: string | null) { const rules: LoadedRule[] = []; const diagnostics: RuleDiagnostic[] = []; @@ -193,8 +275,10 @@ function loadCandidate( deps: EngineDeps, diagnostics: RuleDiagnostic[], projectRoot: string | null, + loadedRuleContent?: Map, + projectMembership?: CandidateProjectMembership, ): (LoadedRule & { matchReason: MatchReason }) | null { - if (!isCandidateWithinProject(candidate, projectRoot)) { + if (!isCandidateWithinProjectCached(candidate, projectRoot, projectMembership)) { diagnostics.push({ severity: "warning", source: candidate.path, @@ -203,22 +287,48 @@ function loadCandidate( return null; } + const cachedContent = loadedRuleContent?.get(candidate.realPath); + if (cachedContent !== undefined) { + return loadedRuleFromContent(candidate, cachedContent, diagnostics); + } + const content = deps.readFile(candidate.path); if (content === null) { + loadedRuleContent?.set(candidate.realPath, null); diagnostics.push({ severity: "warning", source: candidate.path, message: "Unable to read rule file" }); return null; } const parsed = parseRule(content); - if (parsed.diagnostic !== undefined) { - diagnostics.push({ severity: "warning", source: candidate.path, message: parsed.diagnostic }); + const loadedContent = { + frontmatter: parsed.frontmatter, + body: parsed.body, + contentHash: hashContent(parsed.body), + diagnostic: parsed.diagnostic, + } satisfies LoadedRuleContent; + loadedRuleContent?.set(candidate.realPath, loadedContent); + return loadedRuleFromContent(candidate, loadedContent, diagnostics); +} + +function loadedRuleFromContent( + candidate: RuleCandidate, + content: LoadedRuleContent | null, + diagnostics: RuleDiagnostic[], +): (LoadedRule & { matchReason: MatchReason }) | null { + if (content === null) { + diagnostics.push({ severity: "warning", source: candidate.path, message: "Unable to read rule file" }); + return null; + } + + if (content.diagnostic !== undefined) { + diagnostics.push({ severity: "warning", source: candidate.path, message: content.diagnostic }); } return { ...candidate, - frontmatter: parsed.frontmatter, - body: parsed.body, - contentHash: hashContent(parsed.body), + frontmatter: content.frontmatter, + body: content.body, + contentHash: content.contentHash, matchReason: { kind: "no-match" }, }; } @@ -240,6 +350,26 @@ function isCandidateWithinProject(candidate: RuleCandidate, projectRoot: string return relativeRealPath === "" || (!relativeRealPath.startsWith("..") && !isAbsolute(relativeRealPath)); } +function isCandidateWithinProjectCached( + candidate: RuleCandidate, + projectRoot: string | null, + projectMembership: CandidateProjectMembership | undefined, +): boolean { + if (projectMembership === undefined) { + return isCandidateWithinProject(candidate, projectRoot); + } + + const cacheKey = `${projectRoot ?? ""}\0${candidate.realPath}`; + const cached = projectMembership.get(cacheKey); + if (cached !== undefined) { + return cached; + } + + const isWithinProject = isCandidateWithinProject(candidate, projectRoot); + projectMembership.set(cacheKey, isWithinProject); + return isWithinProject; +} + function staticMatchReason(rule: LoadedRule): MatchReason | null { if (rule.frontmatter.alwaysApply === true) { return "alwaysApply"; @@ -314,6 +444,10 @@ function toPosixPath(path: string): string { return path.replaceAll("\\", "/"); } +function uniqueStrings(values: ReadonlyArray): string[] { + return [...new Set(values)]; +} + function storeLastLoad( state: SessionState, rules: ReadonlyArray, diff --git a/src/rules/matcher.ts b/src/rules/matcher.ts index 40bded9..7ebb890 100644 --- a/src/rules/matcher.ts +++ b/src/rules/matcher.ts @@ -14,6 +14,22 @@ export interface MatchResult { reason: MatchReason; } +interface CompiledPatternSet { + positiveMatchers: ReadonlyArray<{ pattern: string; isMatch: PathMatcher }>; + negativeMatchers: ReadonlyArray; +} + +export interface MatcherCacheStats { + entries: number; + compiledPatterns: number; +} + +type PathMatcher = (path: string) => boolean; + +const PICOMATCH_OPTIONS = { bash: true, dot: true }; +const MAX_COMPILED_PATTERN_SET_CACHE_ENTRIES = 256; +const compiledPatternSets = new Map(); + export function matchRule(input: MatcherInput): MatchResult { if (input.isSingleFile) { return { matched: true, reason: "single-file" }; @@ -34,13 +50,9 @@ export function matchRule(input: MatcherInput): MatchResult { normalizePath(input.pathBases.basename), ].filter((pathBase): pathBase is string => pathBase !== undefined); - const positivePatterns = patterns.filter((pattern) => !pattern.startsWith("!")); - const negativePatterns = patterns.filter((pattern) => pattern.startsWith("!")); - const negativeMatchers = negativePatterns.map((pattern) => picomatch(pattern.slice(1), { bash: true, dot: true })); - - for (const pattern of positivePatterns) { - const isMatch = picomatch(pattern, { bash: true, dot: true }); + const { positiveMatchers, negativeMatchers } = compiledPatternSetFor(patterns); + for (const { pattern, isMatch } of positiveMatchers) { for (const pathBase of pathBases) { if (!isMatch(pathBase)) { continue; @@ -71,6 +83,54 @@ export function hashContent(body: string): string { return createHash("sha256").update(body).digest("hex"); } +export function resetMatcherCache(): void { + compiledPatternSets.clear(); +} + +export function getMatcherCacheStats(): MatcherCacheStats { + let compiledPatterns = 0; + for (const patternSet of compiledPatternSets.values()) { + compiledPatterns += patternSet.positiveMatchers.length + patternSet.negativeMatchers.length; + } + + return { entries: compiledPatternSets.size, compiledPatterns }; +} + +function compiledPatternSetFor(patterns: ReadonlyArray): CompiledPatternSet { + const cacheKey = patterns.join("\0"); + const cached = compiledPatternSets.get(cacheKey); + if (cached !== undefined) { + compiledPatternSets.delete(cacheKey); + compiledPatternSets.set(cacheKey, cached); + return cached; + } + + const positiveMatchers: Array<{ pattern: string; isMatch: PathMatcher }> = []; + const negativeMatchers: PathMatcher[] = []; + for (const pattern of patterns) { + if (pattern.startsWith("!")) { + negativeMatchers.push(picomatch(pattern.slice(1), PICOMATCH_OPTIONS)); + continue; + } + + positiveMatchers.push({ pattern, isMatch: picomatch(pattern, PICOMATCH_OPTIONS) }); + } + + const compiledPatternSet = { positiveMatchers, negativeMatchers } satisfies CompiledPatternSet; + setCompiledPatternSet(cacheKey, compiledPatternSet); + return compiledPatternSet; +} + +function setCompiledPatternSet(cacheKey: string, compiledPatternSet: CompiledPatternSet): void { + if (compiledPatternSets.size >= MAX_COMPILED_PATTERN_SET_CACHE_ENTRIES) { + const oldestCacheKey = compiledPatternSets.keys().next().value; + if (oldestCacheKey !== undefined) { + compiledPatternSets.delete(oldestCacheKey); + } + } + compiledPatternSets.set(cacheKey, compiledPatternSet); +} + function normalizePatternList(patterns: string | string[] | undefined): string[] { if (patterns === undefined) { return []; diff --git a/src/rules/types.ts b/src/rules/types.ts index db67552..e1d75e2 100644 --- a/src/rules/types.ts +++ b/src/rules/types.ts @@ -120,7 +120,7 @@ export interface PiRulesConfig { * Per-session in-memory dedup state. * * `staticDedup` keys are `{cwd}::{rulePath}::{contentHash}` strings. - * `dynamicDedup` stores session-scoped `{rulePath}::{contentHash}` strings. + * `dynamicDedup` stores `{targetPath}::{rulePath}::{contentHash}` strings grouped by target file. */ export interface SessionState { cwd: string | undefined; diff --git a/test/cache.test.ts b/test/cache.test.ts index ec8e2c0..abe7498 100644 --- a/test/cache.test.ts +++ b/test/cache.test.ts @@ -118,10 +118,11 @@ describe("markDynamicInjected", () => { it("#given fresh state #when marking dynamic injected first time #then returns true", () => { // given const state = createSessionState(); + const scopeKey = "/workspace/project/src/app.ts"; const rule = makeLoadedRule(); // when - const result = markDynamicInjected(state, rule); + const result = markDynamicInjected(state, scopeKey, rule); // then expect(result).toBe(true); @@ -130,29 +131,30 @@ describe("markDynamicInjected", () => { it("#given marked rule #when marking dynamic injected again same rule #then returns false", () => { // given const state = createSessionState(); + const scopeKey = "/workspace/project/src/app.ts"; const rule = makeLoadedRule(); // when - const firstResult = markDynamicInjected(state, rule); - const secondResult = markDynamicInjected(state, rule); + const firstResult = markDynamicInjected(state, scopeKey, rule); + const secondResult = markDynamicInjected(state, scopeKey, rule); // then expect(firstResult).toBe(true); expect(secondResult).toBe(false); }); - it("#given marked dynamic rule #when marking same rule again #then returns false (session dedup)", () => { + it("#given marked dynamic rule #when marking same rule for a different target path #then returns true", () => { // given const state = createSessionState(); const rule = makeLoadedRule(); // when - const firstResult = markDynamicInjected(state, rule); - const secondResult = markDynamicInjected(state, rule); + const firstResult = markDynamicInjected(state, "/workspace/project/src/app.ts", rule); + const secondResult = markDynamicInjected(state, "/workspace/project/src/api.ts", rule); // then expect(firstResult).toBe(true); - expect(secondResult).toBe(false); + expect(secondResult).toBe(true); }); }); @@ -160,11 +162,12 @@ describe("isInjected", () => { it("#given marked dynamic #when isDynamicInjected #then true", () => { // given const state = createSessionState(); + const scopeKey = "/workspace/project/src/app.ts"; const rule = makeLoadedRule(); - markDynamicInjected(state, rule); + markDynamicInjected(state, scopeKey, rule); // when - const result = isDynamicInjected(state, rule); + const result = isDynamicInjected(state, scopeKey, rule); // then expect(result).toBe(true); @@ -190,7 +193,7 @@ describe("clearSession", () => { const state = createSessionState(); const rule = makeLoadedRule(); markStaticInjected(state, rule); - markDynamicInjected(state, rule); + markDynamicInjected(state, "/workspace/project/src/app.ts", rule); state.loadedRules.push(rule); state.diagnostics.push({ severity: "warning", source: rule.realPath, message: "diagnostic" }); @@ -235,15 +238,16 @@ describe("dedup keys", () => { it("#given dynamicDedupKey #when same args #then deterministic", () => { // given + const scopeKey = "/workspace/project/src/app.ts"; const rulePath = "/workspace/project/AGENTS.md"; const contentHash = "hash"; // when - const firstKey = dynamicDedupKey(rulePath, contentHash); - const secondKey = dynamicDedupKey(rulePath, contentHash); + const firstKey = dynamicDedupKey(scopeKey, rulePath, contentHash); + const secondKey = dynamicDedupKey(scopeKey, rulePath, contentHash); // then - expect(firstKey).toBe("/workspace/project/AGENTS.md::hash"); + expect(firstKey).toBe("/workspace/project/src/app.ts::/workspace/project/AGENTS.md::hash"); expect(secondKey).toBe(firstKey); }); }); diff --git a/test/engine.test.ts b/test/engine.test.ts index 2a82ab5..0613224 100644 --- a/test/engine.test.ts +++ b/test/engine.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_MAX_RESULT_CHARS, DEFAULT_MAX_RULE_CHARS } from "../src/rules/constants.js"; import { createEngine, defaultConfig, type EngineDeps } from "../src/rules/engine.js"; +import { matchRule as defaultMatchRule } from "../src/rules/matcher.js"; import type { LoadedRule, PiRulesConfig, RuleCandidate, RuleSource } from "../src/rules/types.js"; const PROJECT_ROOT = "/workspace/project"; @@ -445,6 +446,172 @@ describe("loadDynamicRules", () => { expect(result.rules).toHaveLength(1); expect(result.rules[0]?.path).toBe(candidate.path); }); + + it("#given target file in nested project #when loadDynamicRules #then nearest target project root is used", () => { + // given + const nestedProjectRoot = `${PROJECT_ROOT}/packages/app`; + const targetPath = `${nestedProjectRoot}/src/index.ts`; + const candidate = makeCandidate({ + path: `${nestedProjectRoot}/.sisyphus/rules/typescript.md`, + realPath: `${nestedProjectRoot}/.sisyphus/rules/typescript.md`, + relativePath: ".sisyphus/rules/typescript.md", + }); + const projectRootCalls: string[] = []; + const deps = { + findProjectRoot: (startPath) => { + projectRootCalls.push(startPath); + return startPath === targetPath ? nestedProjectRoot : PROJECT_ROOT; + }, + findCandidates: ({ projectRoot }) => (projectRoot === nestedProjectRoot ? [candidate] : []), + readFile: () => ruleMarkdown('globs: "src/**/*.ts"', "TypeScript rule."), + extractToolPaths: () => [], + } satisfies EngineDeps; + const engine = createEngine(defaultConfig(), deps); + + // when + const result = engine.loadDynamicRules(PROJECT_ROOT, [targetPath]); + + // then + expect(projectRootCalls).toEqual([targetPath]); + expect(result.rules).toHaveLength(1); + expect(result.rules[0]?.path).toBe(candidate.path); + }); + + it("#given duplicate target paths #when loadDynamicRules #then repeated discovery and parsing work is avoided", () => { + // given + const targetPath = `${PROJECT_ROOT}/src/app.ts`; + const candidate = makeCandidate({ + path: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + realPath: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + relativePath: ".sisyphus/rules/typescript.md", + }); + const counters = { + findProjectRoot: 0, + findCandidates: 0, + readFile: 0, + }; + const deps = { + findProjectRoot: () => { + counters.findProjectRoot += 1; + return PROJECT_ROOT; + }, + findCandidates: () => { + counters.findCandidates += 1; + return [candidate]; + }, + readFile: () => { + counters.readFile += 1; + return ruleMarkdown('globs: "src/**/*.ts"', "TypeScript rule."); + }, + extractToolPaths: () => [], + } satisfies EngineDeps; + const engine = createEngine(defaultConfig(), deps); + + // when + const result = engine.loadDynamicRules(PROJECT_ROOT, [targetPath, targetPath, targetPath]); + + // then + expect(result.rules).toHaveLength(1); + expect(counters).toEqual({ + findProjectRoot: 1, + findCandidates: 1, + readFile: 1, + }); + }); + + it("#given same target and unchanged rule body #when loadDynamicRules is called twice #then match decision is reused", () => { + // given + const targetPath = `${PROJECT_ROOT}/src/app.ts`; + const candidate = makeCandidate({ + path: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + realPath: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + relativePath: ".sisyphus/rules/typescript.md", + }); + let matchRuleCalls = 0; + const deps = { + findProjectRoot: () => PROJECT_ROOT, + findCandidates: () => [candidate], + readFile: () => ruleMarkdown('globs: "src/**/*.ts"', "TypeScript rule."), + extractToolPaths: () => [], + matchRule: (input) => { + matchRuleCalls += 1; + return defaultMatchRule(input); + }, + } satisfies EngineDeps; + const engine = createEngine(defaultConfig(), deps); + + // when + const firstResult = engine.loadDynamicRules(PROJECT_ROOT, [targetPath]); + const secondResult = engine.loadDynamicRules(PROJECT_ROOT, [targetPath]); + + // then + expect(firstResult.rules).toHaveLength(1); + expect(secondResult.rules).toHaveLength(1); + expect(matchRuleCalls).toBe(1); + }); + + it("#given same target and changed rule body #when loadDynamicRules is called again #then match decision is re-evaluated", () => { + // given + const targetPath = `${PROJECT_ROOT}/src/app.ts`; + const candidate = makeCandidate({ + path: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + realPath: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + relativePath: ".sisyphus/rules/typescript.md", + }); + let body = "TypeScript rule."; + let matchRuleCalls = 0; + const deps = { + findProjectRoot: () => PROJECT_ROOT, + findCandidates: () => [candidate], + readFile: () => ruleMarkdown('globs: "src/**/*.ts"', body), + extractToolPaths: () => [], + matchRule: (input) => { + matchRuleCalls += 1; + return defaultMatchRule(input); + }, + } satisfies EngineDeps; + const engine = createEngine(defaultConfig(), deps); + + // when + engine.loadDynamicRules(PROJECT_ROOT, [targetPath]); + body = "Updated TypeScript rule."; + engine.loadDynamicRules(PROJECT_ROOT, [targetPath]); + + // then + expect(matchRuleCalls).toBe(2); + }); + + it("#given unchanged rule and different target files #when loadDynamicRules is called #then cache separates target paths", () => { + // given + const firstTarget = `${PROJECT_ROOT}/src/app.ts`; + const secondTarget = `${PROJECT_ROOT}/src/app.test.ts`; + const candidate = makeCandidate({ + path: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + realPath: `${PROJECT_ROOT}/.sisyphus/rules/typescript.md`, + relativePath: ".sisyphus/rules/typescript.md", + }); + let matchRuleCalls = 0; + const deps = { + findProjectRoot: () => PROJECT_ROOT, + findCandidates: () => [candidate], + readFile: () => ruleMarkdown('globs: ["src/**/*.ts", "!src/**/*.test.ts"]', "TypeScript rule."), + extractToolPaths: () => [], + matchRule: (input) => { + matchRuleCalls += 1; + return defaultMatchRule(input); + }, + } satisfies EngineDeps; + const engine = createEngine(defaultConfig(), deps); + + // when + const firstResult = engine.loadDynamicRules(PROJECT_ROOT, [firstTarget]); + const secondResult = engine.loadDynamicRules(PROJECT_ROOT, [secondTarget]); + + // then + expect(firstResult.rules).toHaveLength(1); + expect(secondResult.rules).toEqual([]); + expect(matchRuleCalls).toBe(2); + }); }); describe("formatting", () => { @@ -479,7 +646,7 @@ describe("session state", () => { const engine = createTestEngine({}, [], new Map()); const rule = makeRule(); engine.markStaticInjected(rule); - engine.markDynamicInjected(rule); + engine.markDynamicInjected("/workspace/project/src/app.ts", rule); // when engine.resetSession("/workspace/other"); @@ -516,14 +683,15 @@ describe("session state", () => { expect(engine.isStaticInjected(rule)).toBe(true); }); - it("#given markDynamicInjected for same rule twice #when both called #then second returns false", () => { + it("#given markDynamicInjected for same rule and target path twice #when both called #then second returns false", () => { // given const engine = createTestEngine({}, [], new Map()); + const scopeKey = "/workspace/project/src/app.ts"; const rule = makeRule(); // when - const firstResult = engine.markDynamicInjected(rule); - const secondResult = engine.markDynamicInjected(rule); + const firstResult = engine.markDynamicInjected(scopeKey, rule); + const secondResult = engine.markDynamicInjected(scopeKey, rule); // then expect(firstResult).toBe(true); diff --git a/test/integration/load-extension.integration.test.ts b/test/integration/load-extension.integration.test.ts index abb019f..8c73af5 100644 --- a/test/integration/load-extension.integration.test.ts +++ b/test/integration/load-extension.integration.test.ts @@ -5,7 +5,7 @@ import piRulesExtension from "../../src/index.js"; import { type CapturedCommand, createFakePi, type FakePiHarness } from "../helpers/fake-pi.js"; const EXPECTED_FLAGS = ["pi-rules-disabled", "pi-rules-mode"]; -const EXPECTED_HANDLERS = ["session_start", "before_agent_start", "tool_result"]; +const EXPECTED_HANDLERS = ["session_start", "session_compact", "before_agent_start", "tool_result"]; const EXPECTED_COMMANDS = ["rules", "reload-rules"]; type RegistrationSurface = { @@ -66,7 +66,7 @@ describe("load extension integration", () => { expect(harness.flags.get("pi-rules-mode")?.options).toMatchObject({ type: "string", default: "both" }); }); - it("#given factory called #when inspecting harness.handlers #then 3 hooks registered: session_start, before_agent_start, tool_result", () => { + it("#given factory called #when inspecting harness.handlers #then 4 hooks registered: session_start, session_compact, before_agent_start, tool_result", () => { // given const harness = registerExtension(); diff --git a/test/integration/sample-project.integration.test.ts b/test/integration/sample-project.integration.test.ts index b281816..5c70cc1 100644 --- a/test/integration/sample-project.integration.test.ts +++ b/test/integration/sample-project.integration.test.ts @@ -104,7 +104,7 @@ describe("sample-project full session integration", () => { const appToolText = textContent(appTool); expect(appTool.content).toHaveLength(2); expect(appTool.content[0]?.text).toBe(readFileSync(appPath, "utf-8")); - expect(appToolText).toContain(`Additional project instructions matched for ${appPath}`); + expect(appToolText).toContain("Additional project instructions matched for apps/web/src/App.tsx"); expect(appToolText).toContain("Prefer `unknown` over `any`. Use exhaustive switch checks."); expect(appToolText).toContain("React components must be functional and prop-typed."); @@ -112,7 +112,7 @@ describe("sample-project full session integration", () => { const apiToolText = textContent(apiTool); expect(apiTool.content).toHaveLength(2); expect(apiTool.content[0]?.text).toBe(readFileSync(apiPath, "utf-8")); - expect(apiToolText).toContain(`Additional project instructions matched for ${apiPath}`); + expect(apiToolText).toContain("Additional project instructions matched for packages/api/src/index.ts"); expect(apiToolText).toContain("Prefer `unknown` over `any`. Use exhaustive switch checks."); expect(apiToolText).not.toContain("React components must be functional and prop-typed."); diff --git a/test/matcher.test.ts b/test/matcher.test.ts index a34b583..54de5ed 100644 --- a/test/matcher.test.ts +++ b/test/matcher.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "vitest"; -import { hashContent, matchRule, normalizeGlobs } from "../src/rules/matcher.js"; +import { + getMatcherCacheStats, + hashContent, + matchRule, + normalizeGlobs, + resetMatcherCache, +} from "../src/rules/matcher.js"; import type { RuleFrontmatter } from "../src/rules/types.js"; const defaultPathBases = { @@ -9,6 +15,43 @@ const defaultPathBases = { }; describe("matchRule", () => { + it("#given same glob set is matched repeatedly #when matching multiple path bases #then compiles each pattern once", () => { + // given + resetMatcherCache(); + const frontmatter: RuleFrontmatter = { globs: ["**/*.ts", "!**/*.test.ts"] }; + const pathBases = { + projectRelative: "src/rules/foo.ts", + scopeRelative: "rules/foo.ts", + basename: "foo.ts", + }; + + // when + for (let index = 0; index < 20; index += 1) { + const result = matchRule({ frontmatter, isSingleFile: false, pathBases }); + expect(result.matched).toBe(true); + } + + // then + expect(getMatcherCacheStats()).toEqual({ entries: 1, compiledPatterns: 2 }); + }); + + it("#given many unique glob sets #when matching repeatedly #then matcher cache stays bounded", () => { + // given + resetMatcherCache(); + + // when + for (let index = 0; index < 300; index += 1) { + matchRule({ + frontmatter: { globs: `src/file-${index}.ts` }, + isSingleFile: false, + pathBases: { projectRelative: `src/file-${index}.ts`, basename: `file-${index}.ts` }, + }); + } + + // then + expect(getMatcherCacheStats().entries).toBeLessThanOrEqual(256); + }); + it("#given single-file rule #when matching any path bases #then always matches with single-file reason", () => { // given const frontmatter: RuleFrontmatter = { globs: "never-matches" };