Skip to content

[Phase 1] feat(chat): 법률 RAG 채팅 계약 테스트 추가 - #17

Merged
HOKAGO-MEMORIES merged 2 commits into
developfrom
phase/1-ai-legal-rag-contract-tests
Jun 22, 2026
Merged

[Phase 1] feat(chat): 법률 RAG 채팅 계약 테스트 추가#17
HOKAGO-MEMORIES merged 2 commits into
developfrom
phase/1-ai-legal-rag-contract-tests

Conversation

@HOKAGO-MEMORIES

@HOKAGO-MEMORIES HOKAGO-MEMORIES commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

변경 내용

  • F-3 법률 RAG용 /api/v1/chat Spring 계약과 인증 테스트 추가
  • backend-ai 법률 RAG 응답 계약 테스트 추가
  • 법률 카드 응답 필드(lawName, articleNo, title, content, score) 정리
  • Maven wrapper PowerShell Target null 접근 오류 수정
  • docs/08_API_SPEC.md에 법률 RAG 응답 예시 추가

연결 이슈

closes #16

테스트

  • backend-ai: .\.venv\Scripts\python.exe -m pytest tests/test_agent_chat.py
  • backend: .\mvnw.cmd -Dtest=ChatControllerTest test

참고

CodeRabbit 자동 리뷰 확인 후 보완 예정입니다.

Summary by CodeRabbit

  • New Features
    • Introduced a new authenticated POST /api/v1/chat endpoint with AI-powered legal consultation.
    • Chat responses now include legalCards with law name, article number, title, content, and relevance score for legal queries.
  • Bug Fixes
    • Added clearer handling for when the AI service is unavailable.
  • Documentation
    • Updated the API spec examples for legal consultation responses and property recommendations.
    • Added Phase 1 contract documentation for the AI legal RAG chat behavior.
  • Tests
    • Expanded coverage to validate legal-intent responses include non-empty legal cards with expected fields.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 239c4e1d-eea5-4adb-a8d8-37ebe7b22fa0

📥 Commits

Reviewing files that changed from the base of the PR and between c311ffb and b46e4ff.

📒 Files selected for processing (6)
  • backend-ai/tests/test_agent_chat.py
  • backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java
  • backend/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.java
  • backend/src/main/resources/application.properties
  • backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java
  • backend/src/test/resources/application-test.properties
🚧 Files skipped from review as they are similar to previous changes (5)
  • backend/src/test/resources/application-test.properties
  • backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java
  • backend/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.java
  • backend-ai/tests/test_agent_chat.py
  • backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java

📝 Walkthrough

Walkthrough

Implements Phase 1 of the F-3 AI legal RAG chat feature. Adds ChatRequest, ChatResponse, and LegalCardResponse DTOs, an AiAgentClient that calls the FastAPI /internal/agent/chat endpoint with configurable timeouts and internal key auth, a ChatController exposing POST /api/v1/chat, and contract tests for both Spring and backend-ai. Updates the API spec, adds a phase plan document, and refactors backend-ai test helpers.

Changes

AI Legal Chat (Phase 1 Contract)

Layer / File(s) Summary
Chat DTOs and error handling
backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatRequest.java, ChatResponse.java, LegalCardResponse.java, backend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.java
ChatRequest record with @NotBlank message and String sessionId. ChatResponse record with intent, message, sessionId, and null-safe defensive copying of properties and legalCards in constructor. LegalCardResponse with five law metadata fields: lawName, articleNo, title, content, score. AI_SERVICE_UNAVAILABLE error code added with BAD_GATEWAY status and Korean message.
AiAgentClient with timeout configuration
backend/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.java
Injects base URL, internal API key, connect timeout, and read timeout (defaults 2000ms and 10000ms); configures SimpleClientHttpRequestFactory and builds RestClient. sendMessage POSTs AgentChatRequest to /internal/agent/chat with X-Internal-Api-Key header, maps AgentChatResponse to ChatResponse, throws ApiException(AI_SERVICE_UNAVAILABLE) on RestClientException or null body.
ChatService and ChatServiceImpl
backend/src/main/java/com/ssafy/salmanhae/service/chat/ChatService.java, ChatServiceImpl.java
ChatService interface declares sendMessage(User, ChatRequest) → ChatResponse. ChatServiceImpl implements it by delegating to aiAgentClient.sendMessage(user.getId(), request).
ChatController and Spring configuration
backend/src/main/java/com/ssafy/salmanhae/controller/chat/ChatController.java, backend/src/main/resources/application.properties, backend/src/test/resources/application-test.properties
ChatController exposes POST /api/v1/chat with @AuthenticationPrincipal User and @Valid ChatRequest, delegates to ChatService.sendMessage, returns ApiResponse<ChatResponse>. Configuration adds ai.agent.base-url (localhost:8000), ai.agent.internal-api-key from INTERNAL_API_KEY env, ai.agent.connect-timeout-ms, ai.agent.read-timeout-ms to both production and test properties.
ChatControllerTest with auth and contract validation
backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java
Integration test class with MockMvc and mocked ChatService. Asserts 401 Unauthorized for unauthenticated POST /api/v1/chat. Asserts 200 OK with LEGAL_CONSULT intent and non-empty legalCards array containing law metadata for authenticated requests. Includes loginAccessToken() helper for JWT signup/login flow.
backend-ai stub and contract tests
backend-ai/app/clients/supabase_client.py, backend-ai/tests/test_agent_chat.py
similarity_search_legal_documents stub returns separate lawName and articleNo fields instead of combined title. Test module adds internal_api_headers() helper reading from get_settings(). Existing internal test updated to use helper. New legal-intent test asserts LEGAL_CONSULT intent, non-empty legalCards array, and expected card fields (lawName, articleNo, title, content, numeric score).
API spec, phase plan, and supporting files
docs/08_API_SPEC.md, phases/ai-legal-rag/phase1-contract-tests.md, backend-ai/README.md, backend/mvnw.cmd
docs/08_API_SPEC.md updates legalCards example from null to a populated array and clarifies response heading. Adds phases/ai-legal-rag/phase1-contract-tests.md defining Phase 1 scope, acceptance criteria (auth contract, legal card contract), and architecture rules. Adds backend-ai/README.md header. Fixes backend/mvnw.cmd PowerShell wrapper-dists path resolution using cached $MAVEN_M2_ITEM with null/zero-element check.

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)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ssafy-salman/salmanhae#10: Introduced the ApiException and ErrorCode infrastructure that AiAgentClient now uses to throw AI_SERVICE_UNAVAILABLE on backend-ai communication failures.

Poem

🐇 Hippity-hop through the legal halls,
A FastAPI friend answers RAG calls,
Spring sends a message, camelCase and bright,
legalCards returned—no longer null in sight,
Phase One is done, the contract stands tall! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding legal RAG chat contract tests for Phase 1, which aligns with the changeset's focus on implementing authentication and response structure tests.
Description check ✅ Passed The description includes all required template sections: 변경 내용 (changes), 연결 이슈 (linked issue #16), and 테스트 (test execution confirmation). The 리뷰 포인트 section was appropriately omitted as there were no specific review points.
Linked Issues check ✅ Passed All four objectives from issue #16 are met: Phase 1 harness file created (phases/ai-legal-rag/phase1-contract-tests.md), Spring Boot /api/v1/chat authentication contract tests added (ChatControllerTest.java), backend-ai legal RAG response contract tests added (test_agent_chat.py), and docs/08_API_SPEC.md updated with legal RAG response examples.
Out of Scope Changes check ✅ Passed Changes are appropriately scoped to Phase 1 contract testing objectives. The Maven wrapper PowerShell fix in mvnw.cmd addresses a technical blocking issue preventing test execution, and all other changes directly support the legal RAG contract testing framework.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch phase/1-ai-legal-rag-contract-tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (3)
backend-ai/tests/test_agent_chat.py (1)

61-69: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Strengthen legal-card contract assertions to cover all standardized fields.

This test currently validates lawName, articleNo, and content, but not title and score, 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 win

Assert all standardized legal card fields in the Spring contract test.

The test currently checks lawName/articleNo only, but this contract’s standardized fields include title, content, and score too.

♻️ 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 win

Make properties deeply immutable in the compact constructor.

List.copyOf(properties) is shallow, so mutable Map elements 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4496816 and c311ffb.

📒 Files selected for processing (17)
  • backend-ai/README.md
  • backend-ai/app/clients/supabase_client.py
  • backend-ai/tests/test_agent_chat.py
  • backend/mvnw.cmd
  • backend/src/main/java/com/ssafy/salmanhae/common/exception/ErrorCode.java
  • backend/src/main/java/com/ssafy/salmanhae/controller/chat/ChatController.java
  • backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatRequest.java
  • backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/ChatResponse.java
  • backend/src/main/java/com/ssafy/salmanhae/model/dto/chat/LegalCardResponse.java
  • backend/src/main/java/com/ssafy/salmanhae/service/chat/AiAgentClient.java
  • backend/src/main/java/com/ssafy/salmanhae/service/chat/ChatService.java
  • backend/src/main/java/com/ssafy/salmanhae/service/chat/ChatServiceImpl.java
  • backend/src/main/resources/application.properties
  • backend/src/test/java/com/ssafy/salmanhae/controller/chat/ChatControllerTest.java
  • backend/src/test/resources/application-test.properties
  • docs/08_API_SPEC.md
  • phases/ai-legal-rag/phase1-contract-tests.md

Comment thread backend/src/main/resources/application.properties Outdated
@HOKAGO-MEMORIES
HOKAGO-MEMORIES merged commit b2b3188 into develop Jun 22, 2026
1 check passed
@HOKAGO-MEMORIES
HOKAGO-MEMORIES deleted the phase/1-ai-legal-rag-contract-tests branch June 22, 2026 04:00
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.

[Phase 1] ai-legal-rag contract-tests

1 participant