Skip to content

Fix/#38#44

Merged
1024andrew merged 3 commits into
devfrom
fix/#38
May 7, 2026
Merged

Fix/#38#44
1024andrew merged 3 commits into
devfrom
fix/#38

Conversation

@1024andrew

Copy link
Copy Markdown
Contributor

유형

  • Feat
  • Fix
  • Design
  • Docs
  • Chore
  • Hotfix

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the chat schema and service logic, including the removal of the source_url field, renaming dormitory to dormitory_name, and adding support for smoking-related queries in the query rewriter. The review identified several improvements: removing the unused user_id field in ChatRequest, renaming response_time to response_time_ms for clarity, adding a trailing comma to the smoking_triggers list for consistency, and fixing a formatting inconsistency in the prompt rules.

Comment thread app/schemas/chat.py
question: str
dormitory: Optional[str] = None
dormitory_name: Optional[str] = None
user_id: Optional[int] = Field(default=None, ge=1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

ChatRequestuser_id 필드가 추가되었으나, chat_service.pyanswer_chat_question 함수에서는 이 필드를 사용하지 않고 세션(chat_session.user_id)에서 사용자 ID를 가져오고 있습니다. 요청 페이로드에 중복된 필드가 포함되지 않도록 삭제하는 것이 좋습니다.

Comment thread app/schemas/chat.py
answer: str
answer_status: str
source_url: str
response_time: int

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

response_time 필드명은 단위(예: ms, s)가 명시되어 있지 않아 모호할 수 있습니다. 서비스 로직에서 밀리초 단위(response_time_ms)를 사용하고 있으므로, 일관성과 명확성을 위해 필드명을 response_time_ms로 변경하는 것을 권장합니다.

Suggested change
response_time: int
response_time_ms: int

answer=answer,
answer_status=answer_status.value,
source_url=source_url,
response_time=response_time_ms,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

ChatResponse의 필드명을 response_time_ms로 변경할 경우, 이에 맞춰 생성자 인자명도 변경해야 합니다.

Suggested change
response_time=response_time_ms,
response_time_ms=response_time_ms,

Comment thread app/services/chat_service.py Outdated
"담배펴",
"흡연구역",
"흡연장",
"흡연가능"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

리스트의 마지막 요소에 쉼표(trailing comma)를 추가하는 것이 좋습니다. 이는 향후 요소를 추가할 때 발생할 수 있는 문자열 자동 결합 버그를 방지하고, 다른 리스트(cooking_triggers 등)와의 일관성을 유지하는 데 도움이 됩니다. (PEP 8 권장 사항)

Suggested change
"흡연가능"
"흡연가능",

13.사용자가 전자레인지, 음식 데우기, 데워먹기, 휴게실 전자레인지 위치를 물으면 휴게실, 공용시설, 전자레인지, 음식 데우기, 정수기, 싱크대 키워드를 포함해라.
14.사용자가 방에서 라면을 먹어도 되는지, 라면을 끓여 먹어도 되는지, 조리, 취사, 라면포트, 전기포트, 전열기구 사용 가능 여부를 물으면 반입금지 물품, 취사행위, 전열기구, 라면포트, 전기포트, 조리 금지, 화재위험 키워드를 포함해라.

15. 사용자가 담배, 흡연, 담배 피울 곳, 담배 필 수 있는 곳, 흡연 가능한 장소를 물으면 흡연구역, 흡연장, 지정 흡연구역, 흡연 가능 장소, 금연구역 키워드를 포함해라.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

프롬프트 내 규칙 번호 "15." 뒤의 공백이 이전 규칙들(12, 13, 14)과 일관되지 않습니다. 일관성을 위해 공백을 제거하는 것이 좋습니다.

Suggested change
15. 사용자가 담배, 흡연, 담배 피울 , 담배 있는 , 흡연 가능한 장소를 물으면 흡연구역, 흡연장, 지정 흡연구역, 흡연 가능 장소, 금연구역 키워드를 포함해라.
15.사용자가 담배, 흡연, 담배 피울 , 담배 있는 , 흡연 가능한 장소를 물으면 흡연구역, 흡연장, 지정 흡연구역, 흡연 가능 장소, 금연구역 키워드를 포함해라.

@1024andrew 1024andrew merged commit afd258d into dev May 7, 2026
1 check passed
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