fix(config-cache): has() no longer inflates hit/miss statistics #498
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,23 @@ class ConfigCache { | |
| * @returns {boolean} True if key exists and is valid | ||
| */ | ||
| has(key) { | ||
| // Check directly instead of delegating to get(), which would | ||
| // increment hits/misses and inflate cache statistics (fixes #497) | ||
| if (!this.cache.has(key)) return false; | ||
|
|
||
| // If timestamp is missing, maps are out-of-sync — treat as invalid | ||
| if (!this.timestamps.has(key)) { | ||
| this.cache.delete(key); | ||
| return false; | ||
| } | ||
|
|
||
| const timestamp = this.timestamps.get(key); | ||
| if (Date.now() - timestamp > this.ttl) { | ||
| this.cache.delete(key); | ||
| this.timestamps.delete(key); | ||
| return false; | ||
| } | ||
| return true; | ||
|
Comment on lines
+75
to
+91
There was a problem hiding this comment. This duplicates expiration/cleanup logic that likely also exists in
Comment on lines
+79
to
+91
There was a problem hiding this comment. Keep Line 79 now treats a missing timestamp as an invalid entry, but Suggested fix get(key) {
if (!this.cache.has(key)) {
this.misses++;
return null;
}
+ if (!this.timestamps.has(key)) {
+ this.cache.delete(key);
+ this.misses++;
+ return null;
+ }
+
const timestamp = this.timestamps.get(key);
const now = Date.now();
// Check if expired
if (now - timestamp > this.ttl) {
this.cache.delete(key);
this.timestamps.delete(key);
this.misses++;
return null;
}Please add a matching regression for As per coding guidelines, Ensure backwards compatibility — core modules are consumed by all agents. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has()only checksthis.cache.has(key)but assumes a correspondingtimestampexists. If the maps ever become out-of-sync (e.g., timestamp missing),Date.now() - timestampbecomesNaNand the entry will be treated as non-expired, returningtrue. Consider treating a missing timestamp as invalid (cleanup +false) or checkingthis.timestamps.has(key)alongsidethis.cache.has(key).