[Phase 4] feat(ai): 분석 결과 기반 답변 생성 - #46
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds grounded answer generation for ChangesGrounded Analysis Answer Generation
Sequence Diagram(s)sequenceDiagram
participant Client
participant generate_answer as LLMClient.generate_answer
participant _generate_live_analysis_answer
participant builder as build_analysis_answer_prompt
participant chat_completions as /chat/completions
participant fallback as AnalysisAnswerService
rect rgba(70, 130, 180, 0.5)
note over generate_answer,_generate_live_analysis_answer: Live LLM path (PRICE/SAFETY_ANALYSIS)
generate_answer->>_generate_live_analysis_answer: AgentState
_generate_live_analysis_answer->>builder: question, analysis_cards, tool_results_json
builder-->>_generate_live_analysis_answer: formatted prompt string
_generate_live_analysis_answer->>chat_completions: POST with prompt
chat_completions-->>_generate_live_analysis_answer: completion text
_generate_live_analysis_answer-->>generate_answer: answer string
end
rect rgba(180, 100, 70, 0.5)
note over generate_answer: Fallback path (None returned or error)
_generate_live_analysis_answer-->>generate_answer: None
alt PRICE_ANALYSIS
generate_answer->>fallback: generate_price_answer(state)
else SAFETY_ANALYSIS
generate_answer->>fallback: generate_safety_answer(state)
end
fallback-->>generate_answer: Korean fact sentence(s) or insufficient data message
end
generate_answer-->>Client: final answer
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🤖 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 240-380: The functions generate_price_analysis_answer,
generate_safety_analysis_answer, and their helper functions _tool_result,
_analysis_card, _combined_metrics, _items, and _format_won contain business
domain logic for fact extraction and response formatting that should not reside
in the LLMClient class which is a transport/client layer concern. Create a new
Service-layer module (e.g., AnalysisService or PriceAnalysisService) and move
all these functions there, then import and call them from LLMClient to maintain
separation of concerns between transport logic and business logic.
- Around line 341-358: The _analysis_card function returns the first matching
card from the analysis_cards list, but since newer cards are appended at the end
of the list, this returns stale data in multi-turn sessions. Change
_analysis_card to return the last matching analysis card instead of the first by
iterating through the list in reverse order or using a different approach to
find the most recent card. Additionally, in the _combined_metrics function, the
current merge order {**result_metrics, **card_metrics} causes older card metrics
to override current tool_result metrics. Reverse the merge order to
{**card_metrics, **result_metrics} so that result_metrics take precedence and
stale card values do not overwrite current facts.
- Around line 186-191: The json.dumps call with tool_results is currently
outside the try block that follows, which means serialization errors will not be
caught by the fallback handler. Move the json.dumps(tool_results,
ensure_ascii=False) call inside the try block along with the
build_analysis_answer_prompt function call so that any serialization errors are
properly handled by the existing fallback mechanism that returns None.
🪄 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: 1bd6c58c-bb00-40eb-a853-fe3ad9d6c874
📒 Files selected for processing (4)
backend-ai/app/clients/llm_client.pybackend-ai/app/rag/prompts.pybackend-ai/tests/test_agent_chat.pybackend-ai/tests/test_analysis_answer_generation.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/analysis_answer_service.py`:
- Around line 18-20: The code uses the `or` operator for numeric field fallbacks
(e.g., `metrics.get("comparableTransactionCount") or len(transactions)`), which
treats valid `0` values as falsy and incorrectly replaces them with default
values. Replace all instances of this pattern with explicit `is None` checks
instead. Specifically, change the comparable_count assignment on line 18 and the
similar numeric field checks in lines 69-74 to use `if metrics.get("fieldName")
is not None` pattern to preserve `0` as a valid metric value.
🪄 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: e02f0a78-0c0e-4d5a-b619-aedb0061b400
📒 Files selected for processing (4)
backend-ai/app/clients/llm_client.pybackend-ai/app/services/__init__.pybackend-ai/app/services/analysis_answer_service.pybackend-ai/tests/test_analysis_answer_generation.py
✅ Files skipped from review due to trivial changes (1)
- backend-ai/app/services/init.py
This was referenced Jun 25, 2026
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.
변경 내용
closes #45
테스트
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests