Skip to content
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

Merge Develop #16

Merged
merged 6 commits into from
Jun 7, 2023
Merged

Merge Develop #16

merged 6 commits into from
Jun 7, 2023

Conversation

siisee11
Copy link
Collaborator

@siisee11 siisee11 commented Jun 7, 2023

No description provided.

siisee11 added 6 commits May 30, 2023 01:39
* feat: tts using ElevenLabs (not streaming) (#10)

* feat: load all files in SOURCE_DOCUMENTS (#11)

* feat: update obisidian loader

* feat: update README
# Conflicts:
#	apps/api/README.md
#	apps/api/ingest.py
@vercel
Copy link

vercel bot commented Jun 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
janot-web 🔄 Building (Inspect) Jun 7, 2023 10:26am

MODEL:
PROMPT:
top_p: 1
temperature: 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위의 코드 패치는 Github actions를 사용하여 pull_request 이벤트가 일어날 때 ChatGPT-CodeReview라는 저장소에서 작동하는 workflow입니다.

리뷰 기능을 위한 권한 부여, 운영 체제 및 작업 단계가 설정되어 있습니다. 또한, prompt, top_p 및 temperature와 같은 추가 환경 변수가 지정됩니다.

# Optional; to run only when a label is attached 주석 아래의 조건문을 풀면 해당 라벨이 붙은 경우에만 코드 리뷰 워크플로우가 실행됩니다.

해당 코드 파일은 보안 관련 문제를 호소하지 않고, 개별 도구와의 통합에 대한 미래적 개선이 가능합니다. 각각의 시나리오에서 적절하게 CSRF(Cross-Site Request Forgery), XSS(Cross-Site Scripting) 등을 예방할 필요성이 있습니다.

{
"name": "apps/llm",
"path": "../apps/llm"
},
{
"name": "apps/web",
"path": "../apps/web"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 지정된 경로를 가지는 "apps/llm"이라는 새 라이브러리를 프로젝트에 추가하는 것입니다. 패치 자체에는 버그나 위험성은 없습니다. 이러한 수정 사항을 제한적으로 수용하거나 테스트 후 전체적으로 병합할 것을 권장합니다. 추가적으로, 일관성을 유지하기 위해 프로젝트에서 "name" 필드에 사용되는 컨벤션을 따르는 것이 좋습니다.

@@ -1,3 +1,5 @@
export OPENAI_API_KEY=your-key
export SUPABASE_BASE_URL=https://zwxffmhaivlzfxonxdwq.supabase.co
export SUPABASE_ANON_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJlZiI6Inp3eGZmbWhhaXZsemZ4b254ZHdxIiwicm9sZSI6ImFub24iLCJpYXQiOjE2ODQwNDAyNjcsImV4cCI6MTk5OTYxNjI2N30.97SWP08kPH05hjMyuswjrqHqQ1-dCHcp_LxN1y3MY70
export ELEVENLABS_API_KEY=your-key
export ELEVENLABS_VOICE_ID=EXAVITQu4vr4xnSDxMaL
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드의 변경점은 환경 변수(OPENAI_API_KEY, SUPABASE_BASE_URL, SUPABASE_ANON_KEY)를 추가하고, 뒤이어 새로운 환경 변수(ELEVENLABS_API_KEY, ELEVENLABS_VOICE_ID)를 추가하는 것입니다. 이전 코드는 수정되지 않았습니다.

위의 정보만으로는 버그 위험이나 개선 제안을 할 수 없습니다. 하지만 이 코드 자체는 안전하고 오류가없으며, 새로운 작업을 위한 환경 변수를 추가하기 위한 일반적인 방법입니다.

@@ -139,6 +139,6 @@ dmypy.json
python.langchain.com/
*vectorstore.pkl

example_data
SOURCE_DOCUMENTS

*.mp3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드는 변경 내역이 명확합니다.

139번째 줄에서 "example_data"를 "SOURCE_DOCUMENTS"로 변경하면서 파일 경로명에 대한 이해를 높였습니다.

오류 위험이나 개선 제안은 여기서 찾을 수 없습니다. 하지만, 전체 코드 범위에 대한 이해가 필요할 수 있습니다.


### Additional Commands
`poetry add <package-name>`

### Env variable
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위의 코드 패치에 대해 간략한 코드 검토를 도와드리겠습니다. 버그 위험 및 개선 제안 사항이 있다면 언제든지 알려주세요.

  • 가상 환경을 실행하고 필요한 패키지를 설치하는 것으로 시작하여, 미리 지정된 'SOURCE_DOCUMENTS' 경로에서 .pdf, .csv, .txt 등의 문서를 위치시키고 ingest 스크립트를 실행합니다.
  • 이후 FASTAPI 서버를 시작하기 위해 make start 명령어를 사용할 수 있으며, Swagger API 문서와 Redoc API 문서에 접근할 수 있습니다.
  • 추가적으로 필요한 패키지는 poetry add <package-name>으로 설치할 수 있으며, 필요에 따라 환경 변수를 설정할 수 있습니다.

코드 상의 큰 문제는 없어 보입니다. 다만, 주석 설명에 '- run ingest.sh'와 같은 부분이 있으나 코드 상에 해당하는 스크립트 파일이 존재하지 않아 혼란스러울 수 있으니 수정해주는 것이 좋을 것 같습니다.


# Define the folder for storing database
SOURCE_DIRECTORY = f"{ROOT_DIRECTORY}/SOURCE_DOCUMENTS"
PERSIST_DIRECTORY = f"{ROOT_DIRECTORY}/DB"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 ROOT_DIRECTORY와 SOURCE_DIRECTORY 및 PERSIST_DIRECTORY 상수를 정의합니다. 이 코드 구문은 파일 경로를 계산하고 파일을 저장하는 데 사용됩니다.

이 코드에서 실수할 위험이 크지 않지만, 여기 몇 가지 개선 방안이 있습니다. 첫째, 변수 이름은 대문자로 표시됩니다. 보통 파이썬에서 대문자는 상수를 나타내므로 이름을 변경하는 것이 좋습니다. 또한 변수 이름은 보다 명확하도록 지정할 수 있습니다. 예를 들어, SOURCE_DOCUMENTS 대신 SOURCE_DIRECTORY라는 좀 더 명확한 이름을 사용할 수 있습니다.

두번째 개선 방안으로는 f-strings 대신 os.path_join() 함수를 사용하여 경로를 조합하는 것입니다. 이렇게 하면 OS와 상관없이 유니버설한 경로를 만들 수 있으며, 쉼표로 결합해야 할 많은 경로 요소가 있는 경우 더 간결하게 쓸 수 있습니다.

따라서 다음과 같이 개선할 수 있습니다:

import os

Define the folder for storing database

root_directory = os.path.dirname(os.path.realpath(file))
source_directory = os.path.join(root_directory, "SOURCE_DIRECTORY")
persist_directory = os.path.join(root_directory, "DB")

if __name__ == "__main__":
# ingest_docs()
ingest_pdf()
ingest_docs()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드는 특정 경로의 파일에서 문서를 로드하고 처리하는 것으로 보입니다. 코드 유지 관리의 측면에서 몇 가지 개선 사항이 있습니다.

  1. 파일 확장명을 하드 코딩하지 마세요.
  2. 중첩 된 if문 대신 dict를 사용하여 파일 로더를 매핑하세요.
  3. print 대신 logging 모듈을 사용하세요.
  4. 코드 구성요소를 별도의 클래스로 추출하세요.

또한 코드는 현재 FAISS 벡터 저장소에 대한 단일 함수(ingest_pdf)만 지원합니다. 이것은 일부 지식 데이터에서만 작동할 수 있습니다. 따라서 기능이 향상될 수 있습니다.

예를 들어, PDF로드 및 전처리와 같은 추가 함수가 필요할 수 있습니다. 문서를 텍스트로 바꾸기 전에 OCR(광학 문자 인식) 등의 추가 선행 처리가 필요한 경우도 있을 수 있습니다.

결론적으로 이 코드의 다양한 측면을 개선하면 더 나은 유지 보수성과 높은 생산성을 얻을 수 있을 것입니다.

return FileResponse(OUTPUT_FILE, media_type="audio/mp3")



@app.websocket("/chat")
async def websocket_endpoint(websocket: WebSocket):
await websocket.accept()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치는 FastAPI를 사용하여 작성된 서버의 특정 기능을 수행하는 함수들로 구성되어 있습니다. 전반적으로 보았을 때, 이 코드는 안전하고 잘 작성되어 있는 것으로 보입니다.

가장 큰 문제는 현재 존재하지 않는 파일을 열려고 시도하고 있다는 것입니다. startup_event() 함수에서 janot_vectorstore.pkl 파일을 열려고 시도하고 있는데, 이 파일은 vectorstore.pkl 파일로 이름이 변경되어 저장된 것 같습니다. 때문에 vectorstore.pkl 파일이 없는 경우 오류가 발생할 것입니다. 이 문제를 해결하기 위해서 startup_event() 함수에서 맞는 파일 이름을 사용해야 합니다.

tts() 함수에서 요청 URL을 하드코딩하고 있습니다. 이것은 나중에 API 업그레이드나 URL이 변경될 경우 에러를 유발할 가능성이 있으므로, 개발환경 및 운영환경 간의 구분 및 설정에 따라 URL 정보를 동적으로 가져오는 것이 좋을 것입니다.

마지막으로, 입력값 검증에 대한 사항이 확인되지 않았습니다. 기존에 해당 서비스를 사용하는 곳에서는 어떤 값들이 들어올 지 알고 있을 수도 있지만, 일반적으로는 입력 값 검증을 수행하여 안정적인 서비스를 구현하는 것이 좋습니다. 이를 위해 FastAPI에서 지원하는 validation 기능을 활용하는 것이 좋을 것입니다.

@@ -176,6 +194,7 @@ export default function Home() {
className="overflow-auto text-center text-xl font-thin tracking-tight text-white"
></div>
</div>
<audio ref={audioRef} controls />
</main>
</div>
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 패치에는 WebSocket을 사용하여 메시지를 처리하고 녹음된 오디오 출력을 재생하는 다른 기능이 포함되어 있습니다.

개선 제안:

  • audioRef는 타입을 명시하는 것이 좋습니다. (예: const audioRef = useRef<HTMLAudioElement>(null);)
  • delay 함수는 사용되지 않기 때문에 삭제할 수 있습니다.
  • enableTTS 상수를 true로 설정하여 음성 합성(api: "http://localhost:9000/tts")이 잘 작동하는지 확인하는 것이 좋습니다.
  • listen 함수가 호출될 때 WebSocket 세션을 초기화해야합니다. 그렇지 않을 경우 'WebSocket already in CLOSING or CLOSED state' 오류가 발생합니다.
    • 이를 위해서 listen 함수에서 ws 변수에 값이 할당되었는지 확인 후 (if(ws) ws.close();) WebSocket 세션을 닫은 다음 다시 열도록 변경할 수 있습니다. setState 함수를 통해 상태값을 관리하는 것이 더 좋을 수도 있습니다.
  • 마지막으로, URL 주소('http://localhost:9000/')는 하드코딩되어 있으며, 환경변수나 구성 파일을 사용하여 가져와서 사용하는 것이 더 좋습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant