Skip to content

Commit 60fb685

Browse files
committed
fix(config-loader): use total requests as denominator for cacheHitRate
The old formula divided cacheHits by loads (disk reads only), which could produce rates above 100% since most requests are cache hits that never touch disk. Use (cacheHits + cacheMisses) as the denominator for a correct percentage. 25 unit tests including 2 regression tests for this fix. Closes #499
1 parent fcfb757 commit 60fb685

2 files changed

Lines changed: 383 additions & 2 deletions

File tree

.aiox-core/core/config/config-loader.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,11 @@ function clearCache() {
221221
function getPerformanceMetrics() {
222222
return {
223223
...performanceMetrics,
224-
cacheHitRate: performanceMetrics.loads > 0
225-
? ((performanceMetrics.cacheHits / performanceMetrics.loads) * 100).toFixed(1) + '%'
224+
// Use total requests (hits + misses) as denominator, not just disk loads.
225+
// `loads` only counts file reads (≈ cache misses), so the old formula
226+
// could exceed 100% and was misleading (fixes #499).
227+
cacheHitRate: (performanceMetrics.cacheHits + performanceMetrics.cacheMisses) > 0
228+
? ((performanceMetrics.cacheHits / (performanceMetrics.cacheHits + performanceMetrics.cacheMisses)) * 100).toFixed(1) + '%'
226229
: '0%',
227230
avgLoadTimeMs: Math.round(performanceMetrics.avgLoadTime),
228231
};
Lines changed: 378 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,378 @@
1+
/**
2+
* Unit tests for config-loader module
3+
*
4+
* Tests the lazy-loading config loader with caching, agent-specific
5+
* section loading, performance metrics, and validation.
6+
*/
7+
8+
const path = require('path');
9+
10+
jest.mock('fs', () => ({
11+
promises: {
12+
readFile: jest.fn(),
13+
},
14+
}));
15+
jest.mock('js-yaml');
16+
17+
const fs = require('fs').promises;
18+
const yaml = require('js-yaml');
19+
20+
const {
21+
loadFullConfig,
22+
loadConfigSections,
23+
loadAgentConfig,
24+
loadMinimalConfig,
25+
preloadConfig,
26+
clearCache,
27+
getPerformanceMetrics,
28+
validateAgentConfig,
29+
getConfigSection,
30+
agentRequirements,
31+
ALWAYS_LOADED,
32+
} = require('../../../.aios-core/core/config/config-loader');
33+
34+
describe('config-loader', () => {
35+
beforeEach(() => {
36+
jest.resetAllMocks();
37+
jest.spyOn(console, 'log').mockImplementation();
38+
jest.spyOn(console, 'error').mockImplementation();
39+
// Clear cache between tests
40+
clearCache();
41+
});
42+
43+
// ============================================================
44+
// Constants
45+
// ============================================================
46+
describe('constants', () => {
47+
test('ALWAYS_LOADED contains core sections', () => {
48+
expect(ALWAYS_LOADED).toContain('frameworkDocsLocation');
49+
expect(ALWAYS_LOADED).toContain('projectDocsLocation');
50+
expect(ALWAYS_LOADED).toContain('devLoadAlwaysFiles');
51+
expect(ALWAYS_LOADED).toContain('lazyLoading');
52+
});
53+
54+
test('agentRequirements maps known agents', () => {
55+
expect(agentRequirements.dev).toBeDefined();
56+
expect(agentRequirements.qa).toBeDefined();
57+
expect(agentRequirements.po).toBeDefined();
58+
expect(agentRequirements.architect).toBeDefined();
59+
expect(agentRequirements.devops).toBeDefined();
60+
});
61+
62+
test('all agents have ALWAYS_LOADED sections', () => {
63+
for (const [, sections] of Object.entries(agentRequirements)) {
64+
for (const required of ALWAYS_LOADED) {
65+
expect(sections).toContain(required);
66+
}
67+
}
68+
});
69+
70+
test('dev agent has specialized sections', () => {
71+
expect(agentRequirements.dev).toContain('pvMindContext');
72+
expect(agentRequirements.dev).toContain('hybridOpsConfig');
73+
});
74+
});
75+
76+
// ============================================================
77+
// loadFullConfig
78+
// ============================================================
79+
describe('loadFullConfig', () => {
80+
test('loads and parses YAML config file', async () => {
81+
const mockConfig = { frameworkDocsLocation: 'docs/', lazyLoading: true };
82+
fs.readFile.mockResolvedValue('yaml content');
83+
yaml.load.mockReturnValue(mockConfig);
84+
85+
const result = await loadFullConfig();
86+
expect(result).toEqual(mockConfig);
87+
expect(fs.readFile).toHaveBeenCalledWith(
88+
path.join('.aios-core', 'core-config.yaml'),
89+
'utf8'
90+
);
91+
});
92+
93+
test('throws on file read error', async () => {
94+
fs.readFile.mockRejectedValue(new Error('ENOENT'));
95+
96+
await expect(loadFullConfig()).rejects.toThrow('Config load failed');
97+
});
98+
99+
test('logs error message on failure', async () => {
100+
fs.readFile.mockRejectedValue(new Error('ENOENT'));
101+
102+
try { await loadFullConfig(); } catch {}
103+
expect(console.error).toHaveBeenCalled();
104+
});
105+
});
106+
107+
// ============================================================
108+
// loadConfigSections
109+
// ============================================================
110+
describe('loadConfigSections', () => {
111+
test('loads requested sections from full config', async () => {
112+
const mockConfig = {
113+
frameworkDocsLocation: 'docs/',
114+
lazyLoading: true,
115+
toolConfigurations: { lint: true },
116+
};
117+
fs.readFile.mockResolvedValue('yaml');
118+
yaml.load.mockReturnValue(mockConfig);
119+
120+
const result = await loadConfigSections(['frameworkDocsLocation', 'lazyLoading']);
121+
expect(result.frameworkDocsLocation).toBe('docs/');
122+
expect(result.lazyLoading).toBe(true);
123+
expect(result.toolConfigurations).toBeUndefined();
124+
});
125+
126+
test('ignores non-existent sections', async () => {
127+
fs.readFile.mockResolvedValue('yaml');
128+
yaml.load.mockReturnValue({ a: 1 });
129+
130+
const result = await loadConfigSections(['a', 'nonExistent']);
131+
expect(result.a).toBe(1);
132+
expect(result.nonExistent).toBeUndefined();
133+
});
134+
135+
test('uses cache on second call', async () => {
136+
const mockConfig = { a: 1, b: 2 };
137+
fs.readFile.mockResolvedValue('yaml');
138+
yaml.load.mockReturnValue(mockConfig);
139+
140+
// First call loads
141+
await loadConfigSections(['a']);
142+
// Second call should use cache
143+
const result = await loadConfigSections(['b']);
144+
expect(result.b).toBe(2);
145+
// readFile should be called only once (cached)
146+
expect(fs.readFile).toHaveBeenCalledTimes(1);
147+
});
148+
});
149+
150+
// ============================================================
151+
// loadAgentConfig
152+
// ============================================================
153+
describe('loadAgentConfig', () => {
154+
test('loads config for known agent', async () => {
155+
const mockConfig = {
156+
frameworkDocsLocation: 'docs/',
157+
projectDocsLocation: 'pdocs/',
158+
devLoadAlwaysFiles: [],
159+
lazyLoading: true,
160+
toolConfigurations: { lint: true },
161+
};
162+
fs.readFile.mockResolvedValue('yaml');
163+
yaml.load.mockReturnValue(mockConfig);
164+
165+
const result = await loadAgentConfig('qa');
166+
expect(result.frameworkDocsLocation).toBe('docs/');
167+
expect(result.toolConfigurations).toEqual({ lint: true });
168+
});
169+
170+
test('falls back to ALWAYS_LOADED for unknown agent', async () => {
171+
const mockConfig = {
172+
frameworkDocsLocation: 'docs/',
173+
projectDocsLocation: 'pdocs/',
174+
devLoadAlwaysFiles: [],
175+
lazyLoading: true,
176+
toolConfigurations: { lint: true },
177+
};
178+
fs.readFile.mockResolvedValue('yaml');
179+
yaml.load.mockReturnValue(mockConfig);
180+
181+
const result = await loadAgentConfig('unknown-agent');
182+
expect(result.frameworkDocsLocation).toBe('docs/');
183+
// Unknown agent should NOT get toolConfigurations (not in ALWAYS_LOADED)
184+
expect(result.toolConfigurations).toBeUndefined();
185+
});
186+
187+
test('logs loading messages', async () => {
188+
fs.readFile.mockResolvedValue('yaml');
189+
yaml.load.mockReturnValue({});
190+
191+
await loadAgentConfig('dev');
192+
expect(console.log).toHaveBeenCalledWith(
193+
expect.stringContaining('@dev')
194+
);
195+
});
196+
});
197+
198+
// ============================================================
199+
// loadMinimalConfig
200+
// ============================================================
201+
describe('loadMinimalConfig', () => {
202+
test('loads only ALWAYS_LOADED sections', async () => {
203+
const mockConfig = {
204+
frameworkDocsLocation: 'docs/',
205+
projectDocsLocation: 'pdocs/',
206+
devLoadAlwaysFiles: ['a.js'],
207+
lazyLoading: true,
208+
toolConfigurations: { lint: true },
209+
pvMindContext: {},
210+
};
211+
fs.readFile.mockResolvedValue('yaml');
212+
yaml.load.mockReturnValue(mockConfig);
213+
214+
const result = await loadMinimalConfig();
215+
expect(result.frameworkDocsLocation).toBe('docs/');
216+
expect(result.lazyLoading).toBe(true);
217+
// Should NOT include non-ALWAYS_LOADED sections
218+
expect(result.toolConfigurations).toBeUndefined();
219+
expect(result.pvMindContext).toBeUndefined();
220+
});
221+
});
222+
223+
// ============================================================
224+
// preloadConfig
225+
// ============================================================
226+
describe('preloadConfig', () => {
227+
test('loads full config into cache', async () => {
228+
fs.readFile.mockResolvedValue('yaml');
229+
yaml.load.mockReturnValue({ a: 1 });
230+
231+
await preloadConfig();
232+
expect(console.log).toHaveBeenCalledWith(expect.stringContaining('Preloading'));
233+
});
234+
});
235+
236+
// ============================================================
237+
// clearCache
238+
// ============================================================
239+
describe('clearCache', () => {
240+
test('forces reload on next call', async () => {
241+
const mockConfig = { a: 1 };
242+
fs.readFile.mockResolvedValue('yaml');
243+
yaml.load.mockReturnValue(mockConfig);
244+
245+
await loadConfigSections(['a']);
246+
clearCache();
247+
await loadConfigSections(['a']);
248+
249+
// After clear, readFile should be called again
250+
expect(fs.readFile).toHaveBeenCalledTimes(2);
251+
});
252+
});
253+
254+
// ============================================================
255+
// getPerformanceMetrics
256+
// ============================================================
257+
describe('getPerformanceMetrics', () => {
258+
test('returns metrics object', () => {
259+
const metrics = getPerformanceMetrics();
260+
expect(metrics).toHaveProperty('loads');
261+
expect(metrics).toHaveProperty('cacheHits');
262+
expect(metrics).toHaveProperty('cacheMisses');
263+
expect(metrics).toHaveProperty('cacheHitRate');
264+
expect(metrics).toHaveProperty('avgLoadTimeMs');
265+
});
266+
267+
test('cacheHitRate is a percentage string', () => {
268+
const metrics = getPerformanceMetrics();
269+
expect(metrics.cacheHitRate).toMatch(/^\d+(\.\d+)?%$/);
270+
});
271+
});
272+
273+
// ============================================================
274+
// validateAgentConfig
275+
// ============================================================
276+
describe('validateAgentConfig', () => {
277+
test('returns valid when all sections exist', async () => {
278+
const mockConfig = {
279+
frameworkDocsLocation: 'docs/',
280+
projectDocsLocation: 'pdocs/',
281+
devLoadAlwaysFiles: [],
282+
lazyLoading: true,
283+
};
284+
fs.readFile.mockResolvedValue('yaml');
285+
yaml.load.mockReturnValue(mockConfig);
286+
287+
const result = await validateAgentConfig('pm');
288+
expect(result.valid).toBe(true);
289+
expect(result.missingSections).toHaveLength(0);
290+
expect(result.agentId).toBe('pm');
291+
});
292+
293+
test('returns invalid when sections are missing', async () => {
294+
fs.readFile.mockResolvedValue('yaml');
295+
yaml.load.mockReturnValue({});
296+
297+
const result = await validateAgentConfig('dev');
298+
expect(result.valid).toBe(false);
299+
expect(result.missingSections.length).toBeGreaterThan(0);
300+
});
301+
302+
test('uses ALWAYS_LOADED for unknown agent', async () => {
303+
fs.readFile.mockResolvedValue('yaml');
304+
yaml.load.mockReturnValue({
305+
frameworkDocsLocation: 'docs/',
306+
projectDocsLocation: 'pdocs/',
307+
devLoadAlwaysFiles: [],
308+
lazyLoading: true,
309+
});
310+
311+
const result = await validateAgentConfig('custom');
312+
expect(result.valid).toBe(true);
313+
expect(result.requiredSections).toEqual(ALWAYS_LOADED);
314+
});
315+
});
316+
317+
// ============================================================
318+
// getConfigSection
319+
// ============================================================
320+
describe('getConfigSection', () => {
321+
test('returns specific section', async () => {
322+
fs.readFile.mockResolvedValue('yaml');
323+
yaml.load.mockReturnValue({ toolConfigurations: { lint: true } });
324+
325+
const result = await getConfigSection('toolConfigurations');
326+
expect(result).toEqual({ lint: true });
327+
});
328+
329+
test('returns undefined for non-existent section', async () => {
330+
fs.readFile.mockResolvedValue('yaml');
331+
yaml.load.mockReturnValue({});
332+
333+
const result = await getConfigSection('nonExistent');
334+
expect(result).toBeUndefined();
335+
});
336+
});
337+
338+
// ============================================================
339+
// BUG #499 regression — cacheHitRate formula
340+
// ============================================================
341+
describe('cacheHitRate formula (fix #499)', () => {
342+
test('cacheHitRate never exceeds 100%', async () => {
343+
// Load once (miss → loads from disk), then hit cache twice
344+
fs.readFile.mockResolvedValue('yaml');
345+
yaml.load.mockReturnValue({ section: 'data' });
346+
await loadConfigSections(['section']); // cache miss
347+
await loadConfigSections(['section']); // cache hit
348+
await loadConfigSections(['section']); // cache hit
349+
350+
const metrics = getPerformanceMetrics();
351+
const rate = parseFloat(metrics.cacheHitRate);
352+
expect(rate).toBeLessThanOrEqual(100);
353+
expect(rate).toBeGreaterThanOrEqual(0);
354+
});
355+
356+
test('cacheHitRate uses total requests as denominator, not disk loads', async () => {
357+
const before = getPerformanceMetrics();
358+
const h0 = before.cacheHits;
359+
const m0 = before.cacheMisses;
360+
361+
// 1 miss + 2 hits
362+
fs.readFile.mockResolvedValue('yaml');
363+
yaml.load.mockReturnValue({ x: 1 });
364+
await loadConfigSections(['x']); // miss
365+
await loadConfigSections(['x']); // hit
366+
await loadConfigSections(['x']); // hit
367+
368+
const after = getPerformanceMetrics();
369+
expect(after.cacheHits - h0).toBe(2);
370+
expect(after.cacheMisses - m0).toBe(1);
371+
372+
// Correct rate: 2/(2+1) = 66.7% (from this batch alone).
373+
// Overall rate should still be <= 100%.
374+
const rate = parseFloat(after.cacheHitRate);
375+
expect(rate).toBeLessThanOrEqual(100);
376+
});
377+
});
378+
});

0 commit comments

Comments
 (0)