-
Notifications
You must be signed in to change notification settings - Fork 83
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
[engine/AI] AI 요약 기능 고도화 #682
Conversation
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.
각 branch를 지원하니 더 완성도 있어 보입니다! feat/579
가 점점 굉장해지고 있군요.
LGTM! 👍
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.
멋집니다! LGTM!! 👏✨🚀
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.
고생하셨습니다 LGTM!!
코드 보면 전체 csm에 대해서 전부 summary를 수행하는 것 같은데,
이럴 경우 성능의 문제는 없을까요?
(라고 comment남기긴 했지만, 별도 branch이니 일단 feature 위주로 진행해주셔도 좋을 것 같습니다.
성능 등은 고려는 하되, 우선순위는 뒷쪽으로~~ 먼저 기능 확인하고 돌리는 것 부터 신경 써주시면 좋을 것 같습니다 😸😸😸)
coverage | ||
.env |
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.
coverage는 지난번에 필요없는 거라고 했던 것 같은데 (맞나용?)
필요없다면 제거해주시면 좋을 것 같습니다~
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.
.env 쪽은 추후에는 vscode setting등으로 넘어갈 수 있는거라고 보면 될까요?
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.
넵 vscode에 보이는 부분 연동하는 과정 추가되면 env는 사라질 것 같습니다!
coverage는 jest --coverage 옵션이 들어있는 커멘드에서 나오는 파일이라 있어야 할 것 같아요!
@@ -75,7 +75,8 @@ export class AnalysisEngine { | |||
if (this.isDebugMode) console.log("stemDict: ", stemDict); | |||
const csmDict = buildCSMDict(commitDict, stemDict, this.baseBranchName, pullRequests); | |||
if (this.isDebugMode) console.log("csmDict: ", csmDict); | |||
const geminiCommitSummary = await getSummary(commitRaws?.slice(-10) ?? []); | |||
const nodes = stemDict.get(this.baseBranchName)?.nodes?.map(({commit}) => commit); | |||
const geminiCommitSummary = await getSummary(nodes ? nodes?.slice(-10) : []); |
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.
이름이 꼭 gemini를 써야 하는 부분이 아니라면 그냥 summary라고 해도 좋을 것 같습니다~
코드 자체가 gemini에 큰 dependency를 보이는 느낌이라서요 😺
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.
요거는 다음브랜치에서 gemini 말고 다른것도 들어가게 될 것 같아서 그때 수정 같이 해보겠습니다! 🙇
@@ -4,9 +4,8 @@ const apiKey = process.env.GEMENI_API_KEY || ''; | |||
const apiUrl = "https://generativelanguage.googleapis.com/v1beta/models/gemini-1.5-flash-latest:generateContent?key="; | |||
|
|||
export async function getSummary(csmNodes: CommitRaw[]) { | |||
const commitMessages = csmNodes.map((csmNode) => csmNode.message).join(', '); | |||
const commitMessages = csmNodes.map((csmNode) => csmNode.message.split('\n')[0]).join(', '); |
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.
(궁금) commit message의 첫 문장만 모아서 요약하는 건지요?
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.
네네! 일부 커밋의 경우 co-author가 커밋으로 같이 들어가있는 경우나 커밋 설명이 너무 길어지면 분석을 잘 못하는 것 같아서 한줄만 잘라서 쓰게 해봤습니다 요거는 프롬프트를 좀더 고쳐봐도 괜찮을 것 같긴 하네요!
Related issue
close #579
Result
Work list
formik 라이브러리에서 브랜치를 변경한 후 나온 출력물입니다.
Discussion
커밋을 가져오는 기준, view에서 보여주는 방법에 대해서는 별도 이슈에서 논의할 예정입니다!