fix: no cache dir created when enableCaching is set to false #191
Conversation
| @@ -16,6 +16,7 @@ export class BaseCache<T extends CacheEntry> { | |||
| private readonly CACHE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 1 week in milliseconds | |||
| private readonly CLEANUP_PROBABILITY = 0.01; // 1% chance | |||
|
|
|||
| protected enableCaching: boolean; | |||
There was a problem hiding this comment.
It doesn't make sense to add this field here. Rather, we should just not init a cache if enableCaching is set to false in LLMProvider and actHandler. It makes more sense to make the cache default undefined in LLMProvider and actHandler, and before a cache operation there, check enableCaching.
| @@ -71,7 +74,7 @@ export class BaseCache<T extends CacheEntry> { | |||
| } | |||
|
|
|||
| protected ensureCacheDirectory(): void { | |||
| if (!fs.existsSync(this.cacheDir)) { | |||
| if (this.enableCaching && !fs.existsSync(this.cacheDir)) { | |||
There was a problem hiding this comment.
This is an antipattern. Since this method is protected, it can only be called from other cache objects. If we just don't init a cache, the directory won't be created
| @@ -273,9 +273,8 @@ export class StagehandActHandler { | |||
|
|
|||
| this.logger({ | |||
| category: "action", | |||
| message: `Clicked element, ${ | |||
There was a problem hiding this comment.
Make sure you format with prettier to keep the diff relevant to code changes
| @@ -27,6 +27,7 @@ | |||
| "devDependencies": { | |||
| "@changesets/changelog-github": "^0.5.0", | |||
| "@changesets/cli": "^2.27.9", | |||
| "@playwright/test": "^1.49.0", | |||
|
Addressing in #197 |
|
Sincerely appreciate your help and time here in trying to resolve this issue! There might be a simpler short-term fix in #197 in the meantime |
why
A
tmp/.cachedirectory is created even when enableCaching is set to false.what changed
test plan