[Phase 1] feat(chat): 법률 RAG 채팅 계약 테스트 추가 - #17
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 (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughImplements Phase 1 of the F-3 AI legal RAG chat feature. Adds ChangesAI Legal Chat (Phase 1 Contract)
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatController
participant ChatServiceImpl
participant AiAgentClient
participant FastAPI as FastAPI /internal/agent/chat
Client->>ChatController: POST /api/v1/chat (Bearer token, message)
ChatController->>ChatServiceImpl: sendMessage(User, ChatRequest)
ChatServiceImpl->>AiAgentClient: sendMessage(userId, ChatRequest)
AiAgentClient->>FastAPI: POST AgentChatRequest + X-Internal-Api-Key
FastAPI-->>AiAgentClient: AgentChatResponse (intent, answer, legalCards)
AiAgentClient-->>ChatServiceImpl: ChatResponse
ChatServiceImpl-->>ChatController: ChatResponse
ChatController-->>Client: ApiResponse(200, ChatResponse)
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
🧹 Nitpick comments (3)
backend-ai/tests/test_agent_chat.py (1)
61-69: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winStrengthen legal-card contract assertions to cover all standardized fields.
This test currently validates
lawName,articleNo, andcontent, but nottitleandscore, so schema regressions on those fields can slip through.Proposed test tightening
body = response.json() assert response.status_code == 200 assert body["intent"] == "LEGAL_CONSULT" assert body["answer"] assert len(body["legalCards"]) >= 1 - assert body["legalCards"][0]["lawName"] == "주택임대차보호법" - assert body["legalCards"][0]["articleNo"] - assert body["legalCards"][0]["content"] + card = body["legalCards"][0] + assert card["lawName"] == "주택임대차보호법" + assert card["articleNo"] + assert card["title"] + assert card["content"] + assert isinstance(card["score"], (int, float))🤖 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/tests/test_agent_chat.py` around lines 61 - 69, The legal card assertions in the test are incomplete and missing validation for the `title` and `score` fields. Add assertions to verify that body["legalCards"][0]["title"] and body["legalCards"][0]["score"] are present and valid, similar to how the existing assertions check for lawName, articleNo, and content. This ensures all standardized fields in the legal card schema are validated and prevents regressions on these fields.backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java (1)
76-83: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAssert all standardized legal card fields in the Spring contract test.
The test currently checks
lawName/articleNoonly, but this contract’s standardized fields includetitle,content, andscoretoo.♻️ Proposed assertion additions
.andExpect(jsonPath("$.data.legalCards", hasSize(1))) .andExpect(jsonPath("$.data.legalCards[0].lawName").value("주택임대차보호법")) - .andExpect(jsonPath("$.data.legalCards[0].articleNo").value("제3조의2")); + .andExpect(jsonPath("$.data.legalCards[0].articleNo").value("제3조의2")) + .andExpect(jsonPath("$.data.legalCards[0].title").value("보증금의 회수")) + .andExpect(jsonPath("$.data.legalCards[0].content").isNotEmpty()) + .andExpect(jsonPath("$.data.legalCards[0].score").value(0.92));🤖 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/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java` around lines 76 - 83, The legal card assertions in the ChatControllerTest are incomplete and only verify lawName and articleNo fields, but the contract requires checking additional standardized fields. Add jsonPath assertions to the andExpect chain to verify that legalCards[0].title and legalCards[0].content are not empty, and that legalCards[0].score contains a valid numeric value. These new assertions should follow the same pattern and be placed alongside the existing lawName and articleNo assertions.backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java (1)
13-15: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMake
propertiesdeeply immutable in the compact constructor.
List.copyOf(properties)is shallow, so mutableMapelements can still be changed after DTO creation and leak into responses.♻️ Proposed fix
public ChatResponse { - properties = properties == null ? List.of() : List.copyOf(properties); + properties = properties == null + ? List.of() + : properties.stream().map(Map::copyOf).toList(); legalCards = legalCards == null ? List.of() : List.copyOf(legalCards); }🤖 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/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java` around lines 13 - 15, The current implementation of the compact constructor in ChatResponse only creates a shallow immutable copy of the properties list using List.copyOf(), which means the Map elements inside can still be mutated after the DTO is created. To fix this and ensure deep immutability, modify the properties assignment to iterate through each Map in the list, convert each Map to an immutable copy using Map.copyOf() or Collections.unmodifiableMap(), and then wrap the resulting collection in List.copyOf(). This ensures both the list and all nested Map objects are immutable and cannot leak mutable state into responses.
🤖 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/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.java`:
- Around line 25-44: The RestClient in the AiAgentClient constructor is built
without explicit connect and read timeouts, and the sendMessage method performs
a blocking call via restClient.post().retrieve().body() that can hang
indefinitely if the backend-ai service stalls. Configure the
RestClient.builder() with explicit connect and read timeouts (e.g., using a
ClientHttpRequestFactory with timeout settings or configuring a
HttpClientFactory with timeout properties), or refactor the sendMessage method
to use asynchronous operations instead of the blocking retrieve().body() call to
prevent servlet thread exhaustion and potential application-wide DoS.
In `@backend/src/main/resources/application.properties`:
- Line 26: The ai.agent.internal-api-key property in application.properties has
a hardcoded fallback value of "change-me" which violates security guidelines for
sensitive credentials. Remove the colon and the default value from the property
definition so that the INTERNAL_API_KEY environment variable becomes mandatory,
causing the application to fail fast at startup if the variable is not set
rather than booting with a known default credential.
In
`@backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java`:
- Around line 104-106: The access token extraction in this method does not
validate that the token was successfully extracted before returning it. After
the line that calls asText() on the accessToken path, add an explicit assertion
to verify that the returned token is not blank or empty. This will ensure that
if the login contract fails or the response structure is incorrect, the test
fails immediately at this extraction point with a clear error message, rather
than later when the token is used in subsequent API calls.
---
Nitpick comments:
In `@backend-ai/tests/test_agent_chat.py`:
- Around line 61-69: The legal card assertions in the test are incomplete and
missing validation for the `title` and `score` fields. Add assertions to verify
that body["legalCards"][0]["title"] and body["legalCards"][0]["score"] are
present and valid, similar to how the existing assertions check for lawName,
articleNo, and content. This ensures all standardized fields in the legal card
schema are validated and prevents regressions on these fields.
In `@backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java`:
- Around line 13-15: The current implementation of the compact constructor in
ChatResponse only creates a shallow immutable copy of the properties list using
List.copyOf(), which means the Map elements inside can still be mutated after
the DTO is created. To fix this and ensure deep immutability, modify the
properties assignment to iterate through each Map in the list, convert each Map
to an immutable copy using Map.copyOf() or Collections.unmodifiableMap(), and
then wrap the resulting collection in List.copyOf(). This ensures both the list
and all nested Map objects are immutable and cannot leak mutable state into
responses.
In
`@backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java`:
- Around line 76-83: The legal card assertions in the ChatControllerTest are
incomplete and only verify lawName and articleNo fields, but the contract
requires checking additional standardized fields. Add jsonPath assertions to the
andExpect chain to verify that legalCards[0].title and legalCards[0].content are
not empty, and that legalCards[0].score contains a valid numeric value. These
new assertions should follow the same pattern and be placed alongside the
existing lawName and articleNo assertions.
🪄 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: 636cb2de-20ae-4f5e-a706-863aa511a42a
📒 Files selected for processing (17)
backend-ai/README.mdbackend-ai/app/clients/supabase_client.pybackend-ai/tests/test_agent_chat.pybackend/mvnw.cmdbackend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.javabackend/src/main/java/com/ssafy/salmanhae/controller/chat/ChatController.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatRequest.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.javabackend/src/main/java/com/ssafy/salmanhae/model/dto/chat/LegalCardResponse.javabackend/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.javabackend/src/main/java/com/ssafy/salmanhae/service/chat/ChatService.javabackend/src/main/java/com/ssafy/salmanhae/service/chat/ChatServiceImpl.javabackend/src/main/resources/application.propertiesbackend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.javabackend/src/test/resources/application-test.propertiesdocs/08_API_SPEC.mdphases/ai-legal-rag/phase1-contract-tests.md
This was referenced Jun 22, 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 #16
테스트
참고
CodeRabbit 자동 리뷰 확인 후 보완 예정입니다.
Summary by CodeRabbit
POST /api/v1/chatendpoint with AI-powered legal consultation.legalCardswith law name, article number, title, content, and relevance score for legal queries.