-
Notifications
You must be signed in to change notification settings - Fork 127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
genai comments #605
base: main
Are you sure you want to change the base?
genai comments #605
Changes from 10 commits
9ece2fc
9597495
8bb6b31
4a21c59
6751eca
0ba03b7
ef57e1b
c0b3997
8e21b53
9317599
fb64388
4838865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,13 +13,17 @@ | |
super() | ||
} | ||
|
||
static byName<K, V>(name: string): JSONLineCache<K, V> { | ||
static byName<K, V>( | ||
name: string, | ||
options?: { snapshot?: boolean } | ||
): JSONLineCache<K, V> { | ||
const { snapshot } = options || {} | ||
name = name.replace(/[^a-z0-9_]/gi, "_") | ||
const key = "cacheKV." + name | ||
if (host.userState[key]) return host.userState[key] | ||
if (!snapshot && host.userState[key]) return host.userState[key] | ||
const r = new JSONLineCache<K, V>(name) | ||
host.userState[key] = r | ||
if (!snapshot) host.userState[key] = r | ||
return r | ||
Check failure on line 26 in packages/core/src/cache.ts GitHub Actions / build
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
|
||
} | ||
|
||
private folder() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { JSONLineCache } from "./cache" | ||
import { COMMENTS_CACHE } from "./constants" | ||
|
||
export interface CommentKey { | ||
uri: string | ||
line: number | ||
source: string | ||
} | ||
|
||
export function commentsCache(options?: { snapshot?: boolean }) { | ||
const cache = JSONLineCache.byName<CommentKey, CommentThread>( | ||
COMMENTS_CACHE, | ||
options | ||
) | ||
return cache | ||
} | ||
|
||
export async function commentsForSource(source: string) { | ||
const cache = commentsCache() | ||
const entries = await cache.entries() | ||
return entries.map(({ val }) => val).filter((c) => c.source === source) | ||
} | ||
Check failure on line 22 in packages/core/src/comments.ts GitHub Actions / build
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import { validateJSONWithSchema } from "./schema" | |
import { YAMLParse } from "./yaml" | ||
import { expandTemplate } from "./expander" | ||
import { resolveLanguageModel } from "./lm" | ||
import { commentsCache, commentsForSource } from "./comments" | ||
|
||
async function resolveExpansionVars( | ||
project: Project, | ||
|
@@ -57,6 +58,7 @@ async function resolveExpansionVars( | |
secrets[secret] = value | ||
} else trace.error(`secret \`${secret}\` not found`) | ||
} | ||
const comments = await commentsForSource(template.id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might have forgotten to use 'await' before calling the async function 'commentsForSource'.
|
||
const res: Partial<ExpansionVariables> = { | ||
dir: ".", | ||
files, | ||
|
@@ -67,6 +69,7 @@ async function resolveExpansionVars( | |
}, | ||
vars: attrs, | ||
secrets, | ||
comments, | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variable
|
||
return res | ||
} | ||
|
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.
The function
byName
does not handle the case whenoptions
is undefined. This could lead to a TypeError when trying to destructuresnapshot
fromoptions
. Consider adding a default value foroptions
in the function parameters or a check foroptions
before destructuring. 🛠️