Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Last reviewed commit: f4dc10a |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Contributor
|
Size Change: +2.31 kB (+0.03%) Total Size: 6.65 MB
ℹ️ View Unchanged
|
carlos-marchal-ph
approved these changes
Mar 6, 2026
Comment on lines
82
to
+85
|
|
||
| private getCacheKey(name: string, version?: number): string { | ||
| return version === undefined ? `${name}::latest` : `${name}::version:${version}` | ||
| } |
Contributor
There was a problem hiding this comment.
I would document that we enforce cache naming in the backend that prevents using :::
Suggested change
| private getCacheKey(name: string, version?: number): string { | |
| return version === undefined ? `${name}::latest` : `${name}::version:${version}` | |
| } | |
| // :: is restricted from use in cache names in the backend, so no chance of accidental collision | |
| private getCacheKey(name: string, version?: number): string { | |
| return version === undefined ? `${name}::latest` : `${name}::version:${version}` | |
| } |
Still a bit brittle since we could change that in the backend without realising this code exists, the ideal solution would be to use a Map<string, Map<number | undefined, CachedPrompt>> as the cache, mimicking the Python approach of a tuple as a key.
Comment on lines
+176
to
+182
| const latestKey = this.getCacheKey(name) | ||
| const versionPrefix = `${name}::version:` | ||
| for (const key of this.cache.keys()) { | ||
| if (key === latestKey) { | ||
| this.cache.delete(key) | ||
| continue | ||
| } |
Contributor
There was a problem hiding this comment.
The approach described in my previous comment would also make clearing all versions for a prompt a one liner without string manipulation.
| * @param name - Optional prompt name to clear. If not provided, clears all cached prompts. | ||
| * @param name - Optional prompt name to clear. If provided, clears all cached versions for that prompt. | ||
| */ | ||
| clearCache(name?: string): void { |
Contributor
There was a problem hiding this comment.
Shouldn't this be clearCache(name?: string, version?: number): void for parity with Python's API?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
versionoption toPrompts.get()so callers can request a specific published prompt version