Skip to content

[Phase 3] feat(ai): 법률 RAG 검색기 구현 - #21

Merged
HOKAGO-MEMORIES merged 3 commits into
developfrom
phase/3-legal-rag-retriever
Jun 22, 2026
Merged

[Phase 3] feat(ai): 법률 RAG 검색기 구현#21
HOKAGO-MEMORIES merged 3 commits into
developfrom
phase/3-legal-rag-retriever

Conversation

@HOKAGO-MEMORIES

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

Copy link
Copy Markdown
Contributor

변경 사항

  • F-3 법률 RAG retriever를 embedding client + Supabase pgvector client 경계로 전환
  • production client의 법률 카드 stub 제거
  • legal row를 chat contract의 legalCards 필드로 정규화
  • legal_rag node tool metadata를 실제 pgvector source로 갱신
  • 외부 네트워크/DB 없이 fake client로 검증하는 retriever 테스트 추가
  • embedding 환경변수 예시 추가

테스트

  • cd backend-ai && .\.venv\Scripts\python.exe -m pytest tests
  • git diff --check

Closes #20

Summary by CodeRabbit

Release Notes

  • New Features
    • Enabled real pgvector-based similarity search for legal document retrieval (replacing stubbed results)
    • Added query embedding + two-stage retrieval pipeline with consistent legal card normalization
    • Added configurable embedding and database timeout settings, and updated Legal RAG tool metadata to reflect the real search source
  • Bug Fixes
    • Improved handling of empty/whitespace queries and enforced safe topK clamping
  • Tests
    • Added unit tests for retrieval, normalization, and timeout behavior; updated chat tests and fixtures
  • Documentation
    • Added Phase 3 planning notes for the Legal RAG retriever backend boundary

@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: 742183ee-cd2d-4642-8e59-ad256a253f17

📥 Commits

Reviewing files that changed from the base of the PR and between 904dbb7 and c3c3ab5.

📒 Files selected for processing (2)
  • backend-ai/.env.example
  • backend-ai/tests/test_legal_retriever.py
✅ Files skipped from review due to trivial changes (1)
  • backend-ai/.env.example
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend-ai/tests/test_legal_retriever.py

📝 Walkthrough

Walkthrough

Replaces the hardcoded legal RAG stub with a real two-stage pipeline: a new EmbeddingClient generates query vectors via HTTP, SupabaseVectorClient executes pgvector similarity search against public.legal_document_chunks, and LegalRetriever orchestrates both through protocol-based dependency injection with input validation and row normalization.

Changes

Legal RAG Retriever Pipeline

Layer / File(s) Summary
Config, environment variables, and protocol contracts
backend-ai/.env.example, backend-ai/app/core/config.py, backend-ai/app/rag/retriever.py
Adds EMBEDDING_API_KEY, EMBEDDING_BASE_URL, SUPABASE_CONNECT_TIMEOUT_SECONDS, and SUPABASE_STATEMENT_TIMEOUT_MS to Settings and .env.example; defines EmbeddingClientProtocol and VectorClientProtocol as DI contracts for the two new client boundaries.
EmbeddingClient HTTP boundary
backend-ai/app/clients/embedding_client.py
New EmbeddingClient reads optional config from constructor args or get_settings(), normalizes base_url, and embed_query validates input, POSTs to /embeddings with bearer auth, validates JSON response structure, and returns list[float].
SupabaseVectorClient pgvector implementation
backend-ai/app/clients/supabase_client.py
Replaces hardcoded stub with real psycopg query using to_pgvector_literal; similarity_search_legal_documents signature changes from query: str to query_embedding: list[float]; constructor accepts optional connection and statement timeout parameters.
LegalRetriever two-stage pipeline and row normalization
backend-ai/app/rag/retriever.py, backend-ai/app/graph/nodes/legal_rag.py
LegalRetriever.__init__ gains DI parameters for embedding and vector clients plus max_top_k; retrieve() trims/validates query, clamps top_k, embeds, searches, and normalizes rows via normalize_legal_card(). The legal_rag node source label changes from supabase-pgvector-stub to supabase-pgvector.
Unit tests with fake clients and agent chat integration
backend-ai/tests/test_legal_retriever.py, backend-ai/tests/test_agent_chat.py
Adds test_legal_retriever.py with FakeEmbeddingClient/FakeVectorClient covering normalization, blank-query short-circuit, top_k clamping, node metadata, and timeout configuration. Updates agent-chat test to monkeypatch LegalRetriever and refresh agent graph cache.
Phase 3 blueprint
phases/ai-legal-rag/phase3-legal-rag-retriever.md
New implementation plan defining scope, architecture constraints, required files, and done-when criteria for this retriever phase.

Sequence Diagram

sequenceDiagram
  participant Client as API Client
  participant legal_rag_node as legal_rag Node
  participant LegalRetriever
  participant EmbeddingClient
  participant SupabaseVectorClient as Supabase pgvector

  Client->>legal_rag_node: chat message (legal question)
  legal_rag_node->>LegalRetriever: retrieve(query, top_k)
  LegalRetriever->>LegalRetriever: trim/validate query, clamp top_k
  LegalRetriever->>EmbeddingClient: embed_query(query)
  EmbeddingClient->>EmbeddingClient: POST /embeddings (bearer auth)
  EmbeddingClient-->>LegalRetriever: list[float] vector
  LegalRetriever->>SupabaseVectorClient: similarity_search_legal_documents(embedding, top_k)
  SupabaseVectorClient->>SupabaseVectorClient: SQL cosine similarity on legal_document_chunks
  SupabaseVectorClient-->>LegalRetriever: list[dict] raw rows
  LegalRetriever->>LegalRetriever: normalize_legal_card(row) for each row
  LegalRetriever-->>legal_rag_node: normalized legal cards
  legal_rag_node-->>Client: legal_cards + tool_results{topK, source}
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ssafy-salman/salmanhae#17: Both PRs modify backend-ai/tests/test_agent_chat.py to validate that /internal/agent/chat returns intent=LEGAL_CONSULT with non-empty legalCards containing expected legal-card metadata fields.
  • ssafy-salman/salmanhae#19: Supabase pgvector retrieval now queries public.legal_document_chunks using the stored embedding field, which depends on the Phase 2 migration and chunk ingestion structure from that PR.

Poem

🐇 Hoppity hop, the stub is gone!
A vector query now carries on,
With embeddings bright and pgvector true,
Fake clients help us test it through.
The bunny RAGs and searches right —
Real legal cards shine in the light! ✨

🚥 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 identifies the main change as Phase 3 implementation of a legal RAG retriever with appropriate specificity and relevance to the changeset.
Description check ✅ Passed The description includes all required template sections: summary of changes, linked issue reference, and testing verification steps. All mandatory information is present and complete.
Linked Issues check ✅ Passed All coding requirements from issue #20 are addressed: LegalRetriever tests added, embedding client boundary implemented, pgvector client created, legal_rag node updated, and tests verified without external dependencies.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #20 scope: configuration updates, client implementations, retriever logic, graph node updates, and test coverage. No out-of-scope changes detected.

✏️ 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/3-legal-rag-retriever

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: 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/clients/supabase_client.py`:
- Around line 33-35: The psycopg.connect and cursor.execute calls in the
similarity search operation lack explicit timeout parameters, which can cause
indefinite blocking during transient DB or network issues. Add a timeout
parameter to the psycopg.connect call when establishing the connection with
self.database_url to set a connection timeout, and optionally set a query
timeout on the cursor.execute call to ensure the similarity search query cannot
hang indefinitely. This will prevent requests from being blocked by transient
database or network stalls.
🪄 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: 83a6e2f1-ee73-4b15-bf5a-815dd22f068b

📥 Commits

Reviewing files that changed from the base of the PR and between e6351f7 and cd30e7b.

📒 Files selected for processing (9)
  • backend-ai/.env.example
  • backend-ai/app/clients/embedding_client.py
  • backend-ai/app/clients/supabase_client.py
  • backend-ai/app/core/config.py
  • backend-ai/app/graph/nodes/legal_rag.py
  • backend-ai/app/rag/retriever.py
  • backend-ai/tests/test_agent_chat.py
  • backend-ai/tests/test_legal_retriever.py
  • phases/ai-legal-rag/phase3-legal-rag-retriever.md

Comment thread backend-ai/app/clients/supabase_client.py Outdated

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

🧹 Nitpick comments (2)
backend-ai/.env.example (1)

5-6: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Reorder the new timeout keys to keep the example env file lint-clean.

dotenv-linter expects SUPABASE_CONNECT_TIMEOUT_SECONDS before SUPABASE_DB_URL, so this placement will keep generating a warning in CI.

🤖 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/.env.example` around lines 5 - 6, The dotenv-linter expects
environment variables to be in a specific order, with
SUPABASE_CONNECT_TIMEOUT_SECONDS appearing before SUPABASE_DB_URL. Reorder the
two timeout configuration lines (SUPABASE_CONNECT_TIMEOUT_SECONDS and
SUPABASE_STATEMENT_TIMEOUT_MS) in the .env.example file to appear before
SUPABASE_DB_URL so the file passes linting without warnings.

Source: Linters/SAST tools

backend-ai/tests/test_legal_retriever.py (1)

125-167: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding a defensive check before indexing calls["executes"].

The test directly indexes calls["executes"][0] at line 167 without verifying the list has at least one element. While the current implementation guarantees this, adding a length assertion would make test failures clearer if the implementation changes.

♻️ Optional defensive assertion
     client.similarity_search_legal_documents([0.1, 0.2], top_k=2)
 
     assert calls["connect_kwargs"]["connect_timeout"] == 7
+    assert len(calls.get("executes", [])) >= 1, "Expected at least one execute call"
     assert calls["executes"][0] == ("set local statement_timeout = %s", (3000,))
🤖 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_legal_retriever.py` around lines 125 - 167, In the
test_supabase_vector_client_sets_connection_and_statement_timeouts function, add
a defensive assertion before accessing calls["executes"][0] to verify that the
list contains at least one element. This will make test failures clearer if the
implementation changes and the similarity_search_legal_documents method no
longer executes the expected SQL statement. Add the assertion immediately after
the client.similarity_search_legal_documents call and before the assert
statements that check the execute call parameters.
🤖 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.

Nitpick comments:
In `@backend-ai/.env.example`:
- Around line 5-6: The dotenv-linter expects environment variables to be in a
specific order, with SUPABASE_CONNECT_TIMEOUT_SECONDS appearing before
SUPABASE_DB_URL. Reorder the two timeout configuration lines
(SUPABASE_CONNECT_TIMEOUT_SECONDS and SUPABASE_STATEMENT_TIMEOUT_MS) in the
.env.example file to appear before SUPABASE_DB_URL so the file passes linting
without warnings.

In `@backend-ai/tests/test_legal_retriever.py`:
- Around line 125-167: In the
test_supabase_vector_client_sets_connection_and_statement_timeouts function, add
a defensive assertion before accessing calls["executes"][0] to verify that the
list contains at least one element. This will make test failures clearer if the
implementation changes and the similarity_search_legal_documents method no
longer executes the expected SQL statement. Add the assertion immediately after
the client.similarity_search_legal_documents call and before the assert
statements that check the execute call parameters.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 900b4a18-964d-4dc5-9eb9-31d21f26e38a

📥 Commits

Reviewing files that changed from the base of the PR and between cd30e7b and 904dbb7.

📒 Files selected for processing (4)
  • backend-ai/.env.example
  • backend-ai/app/clients/supabase_client.py
  • backend-ai/app/core/config.py
  • backend-ai/tests/test_legal_retriever.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend-ai/app/clients/supabase_client.py

@HOKAGO-MEMORIES
HOKAGO-MEMORIES merged commit fd871ff into develop Jun 22, 2026
1 check passed
@HOKAGO-MEMORIES
HOKAGO-MEMORIES deleted the phase/3-legal-rag-retriever branch June 22, 2026 04:49
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 3] legal-rag retriever

1 participant