fix(chat-ai): AI 채팅 매물 검색 품질 개선 (#105) - #107
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThe PR adds optional sort and limit extraction for property searches, changes query ordering to support ascending price searches, updates won-based price formatting in backend and frontend responses, and tightens analysis-answer prompts to avoid exposing internal field names. ChangesProperty search sorting, limits, and price display
Analysis answer prompt hardening
Sequence Diagram(s)sequenceDiagram
participant User
participant llm_client
participant property_search
participant SupabaseVectorClient
participant public_properties
User->>llm_client: asks for property search with sort_by and limit
llm_client->>property_search: extracted criteria
property_search->>property_search: compute effective_limit
property_search->>SupabaseVectorClient: search_properties(limit=effective_limit)
SupabaseVectorClient->>public_properties: ORDER BY and LIMIT query
public_properties-->>SupabaseVectorClient: matching rows
SupabaseVectorClient-->>property_search: properties
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend-ai/app/clients/llm_client.py`:
- Around line 325-329: The price formatting in llm_client.py is using floor
truncation via int(...) // 10000, which conflicts with the frontend’s toMan
rounding behavior. Update the price_str construction in the tx handling block so
the backend uses the same rounding rule as frontend/src/views/Chatbot.vue,
keeping the displayed won-to-man amount consistent across the LLM summary and
property card.
In `@backend-ai/app/graph/nodes/property_search.py`:
- Around line 14-20: The `effective_limit` handling in `property_search` is
unbounded and can pass oversized or negative values into `search_properties`.
After parsing `user_limit` in the existing `try` block, clamp it to a positive
minimum and a reasonable maximum, and keep the fallback behavior for invalid
inputs; use the `criteria.get("limit")` / `criteria.get("sort_by")` logic as the
entry point and ensure `int(user_limit)` never reaches `LIMIT` outside the safe
range.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: aa8e4cb6-7c57-4495-99ea-e1f42e64dfec
📒 Files selected for processing (5)
backend-ai/app/clients/llm_client.pybackend-ai/app/clients/supabase_client.pybackend-ai/app/graph/nodes/property_search.pybackend-ai/app/rag/prompts.pyfrontend/src/views/Chatbot.vue
Comment on lines
+325
to
+329
| price_str = f"{int(deposit) // 10000:,}/{int(rent) // 10000:,}만원" | ||
| elif tx == "전세" and deposit is not None: | ||
| price_str = f"전세 {int(deposit):,}만원" | ||
| price_str = f"전세 {int(deposit) // 10000:,}만원" | ||
| elif tx == "매매" and price is not None: | ||
| price_str = f"매매 {int(price):,}만원" | ||
| price_str = f"매매 {int(price) // 10000:,}만원" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Won→만원 rounding differs from the frontend, causing inconsistent prices for the same property.
Here the backend truncates with int(...) // 10000 (floor), while frontend/src/views/Chatbot.vue (toMan) uses Math.round(... / 10000). For an amount like 5,007,000원 the LLM summary would show 500만원 but the card would show 501만원. Align both surfaces on one rule (rounding is more intuitive for money).
♻️ Use rounding to match the frontend
- if tx == "월세" and deposit is not None and rent is not None:
- price_str = f"{int(deposit) // 10000:,}/{int(rent) // 10000:,}만원"
- elif tx == "전세" and deposit is not None:
- price_str = f"전세 {int(deposit) // 10000:,}만원"
- elif tx == "매매" and price is not None:
- price_str = f"매매 {int(price) // 10000:,}만원"
+ if tx == "월세" and deposit is not None and rent is not None:
+ price_str = f"{round(int(deposit) / 10000):,}/{round(int(rent) / 10000):,}만원"
+ elif tx == "전세" and deposit is not None:
+ price_str = f"전세 {round(int(deposit) / 10000):,}만원"
+ elif tx == "매매" and price is not None:
+ price_str = f"매매 {round(int(price) / 10000):,}만원"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend-ai/app/clients/llm_client.py` around lines 325 - 329, The price
formatting in llm_client.py is using floor truncation via int(...) // 10000,
which conflicts with the frontend’s toMan rounding behavior. Update the
price_str construction in the tx handling block so the backend uses the same
rounding rule as frontend/src/views/Chatbot.vue, keeping the displayed
won-to-man amount consistent across the LLM summary and property card.
Comment on lines
+14
to
+20
| user_limit = criteria.get("limit") | ||
| try: | ||
| effective_limit = int(user_limit) if user_limit else ( | ||
| _SORTED_LIMIT if criteria.get("sort_by") else _DEFAULT_LIMIT | ||
| ) | ||
| except (TypeError, ValueError): | ||
| effective_limit = _DEFAULT_LIMIT |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Clamp effective_limit to a sane range.
int(user_limit) is used unbounded. Two problems flow from an LLM-extracted value:
- A large value (e.g. user says "1000개") is passed straight to
LIMIT, allowing an oversized scan/result set. - A negative value (e.g.
-5) is truthy, soint("-5") → -5reachessearch_properties; Postgres rejectsLIMIT -5, thetry/exceptin this node swallows it, and the user silently gets zero results instead of their search.
Bound the value to a maximum and a positive floor.
🛡️ Proposed clamp
+_MAX_LIMIT = 50
+
def property_search(state: AgentState) -> AgentState:
message = state["message"]
criteria = LLMClient().extract_property_criteria(message)
user_limit = criteria.get("limit")
try:
- effective_limit = int(user_limit) if user_limit else (
- _SORTED_LIMIT if criteria.get("sort_by") else _DEFAULT_LIMIT
- )
+ if user_limit:
+ effective_limit = max(1, min(int(user_limit), _MAX_LIMIT))
+ else:
+ effective_limit = _SORTED_LIMIT if criteria.get("sort_by") else _DEFAULT_LIMIT
except (TypeError, ValueError):
effective_limit = _DEFAULT_LIMIT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| user_limit = criteria.get("limit") | |
| try: | |
| effective_limit = int(user_limit) if user_limit else ( | |
| _SORTED_LIMIT if criteria.get("sort_by") else _DEFAULT_LIMIT | |
| ) | |
| except (TypeError, ValueError): | |
| effective_limit = _DEFAULT_LIMIT | |
| _MAX_LIMIT = 50 | |
| user_limit = criteria.get("limit") | |
| try: | |
| if user_limit: | |
| effective_limit = max(1, min(int(user_limit), _MAX_LIMIT)) | |
| else: | |
| effective_limit = _SORTED_LIMIT if criteria.get("sort_by") else _DEFAULT_LIMIT | |
| except (TypeError, ValueError): | |
| effective_limit = _DEFAULT_LIMIT |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend-ai/app/graph/nodes/property_search.py` around lines 14 - 20, The
`effective_limit` handling in `property_search` is unbounded and can pass
oversized or negative values into `search_properties`. After parsing
`user_limit` in the existing `try` block, clamp it to a positive minimum and a
reasonable maximum, and keep the fallback behavior for invalid inputs; use the
`criteria.get("limit")` / `criteria.get("sort_by")` logic as the entry point and
ensure `int(user_limit)` never reaches `LIMIT` outside the safe range.
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
1 participant
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.
변경 내용
sort_by추가 — "가장 싼", "저렴한" 등 저가 정렬 의도 감지 시 거래유형별 가격 오름차순 정렬 (SALE→price, JEONSE→deposit, MONTHLY_RENT→monthly_rent)limit추가 — "1개 보여줘" 등 명시적 개수 요청 반영, 정렬 요청 시 기본 5개·일반 검색 시 기본 10개로 고정 20개 문제 해결연결 이슈
closes #105
테스트
리뷰 포인트
supabase_client.search_properties의 ORDER BY 절이 f-string으로 동적 생성됨 —order_clause는 코드 내 상수이므로 SQL injection 위험 없음sort_by,limit는field_map/range_map에 없는 키라 WHERE 조건 생성에 영향 없음Summary by CodeRabbit
New Features
Bug Fixes