fix(chat): repair chain wiring and context handling#770
Conversation
- [x] construct real chat chains with the right dependencies\n- [x] normalize and propagate request context into NL query and RAG paths\n- [x] make RAG structured lookups work on Postgres placeholders\n- [x] add adapter-level regression coverage for chat routing
🤖 Keepalive Loop StatusPR #770 | Agent: Codex | Iteration 0/5 Current State
🔍 Failure Classification| Error type | infrastructure | |
Keepalive Work Log (click to expand)
|
There was a problem hiding this comment.
Pull request overview
Repairs chat-chain construction and context propagation so the chat API routes into the “real” chain contracts and carries manager/date/cusip filters into NL-to-SQL and RAG flows, with regression tests around adapter routing and Postgres-safe structured queries.
Changes:
- Update
api/chat.pychain building/running to normalize context and route filing-summary / holdings-analysis via their intended contracts. - Enhance
chains/rag_search.pystructured retrieval to support Postgres placeholders + optional table probing and merge explicit context filters. - Extend
chains/nl_query.pyprompts to include context filters; add regression tests for prompt/filter behavior and Postgres placeholder usage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
api/chat.py |
Normalizes request context, adjusts chain selection, and rewires chain instantiation/invocation. |
chains/rag_search.py |
Adds DB-dialect placeholders, optional table checks, and context merge into structured search. |
chains/nl_query.py |
Injects context filters into the NL-to-SQL prompt text. |
tests/test_chat_api.py |
Adds regression tests for direct endpoints wiring + auto fallback routing. |
tests/test_rag_search_chain.py |
Adds regression tests for Postgres placeholder usage and explicit context filters. |
tests/test_nl_query_chain.py |
Adds regression test ensuring context filters appear in the prompt. |
You can also share your feedback on Copilot code review. Take the survey.
| if chain_name == "filing_summary": | ||
| filing_id = normalized_context.get("filing_id") | ||
| try: | ||
| if filing_id is None: | ||
| raise TypeError("missing filing_id") | ||
| result = chain.run(int(cast(Any, filing_id))) | ||
| except (TypeError, ValueError) as exc: |
| elif chain_name == "holdings_analysis": | ||
| result = chain.run( | ||
| question, | ||
| manager_ids=_normalize_manager_ids(normalized_context.get("manager_ids")) or None, | ||
| cusips=_normalize_cusips(normalized_context.get("cusips")) or None, | ||
| date_range=_parse_date_range(normalized_context.get("date_range")), | ||
| ) |
| def _build_chain(chain_name: str, client_info: Any): | ||
| """Instantiate a chain by name, falling back to local stubs when missing.""" | ||
| module_path, class_name = DIRECT_CHAIN_PATHS[chain_name] | ||
| try: | ||
| module = importlib.import_module(module_path) | ||
| chain_cls = getattr(module, class_name) | ||
| except Exception: | ||
| chain_cls = FALLBACK_CHAINS[chain_name] | ||
| return FALLBACK_CHAINS[chain_name]() | ||
|
|
||
| db_conn = connect_db() | ||
| try: | ||
| return chain_cls(client_info.client if client_info else None) | ||
| if chain_name == "filing_summary": | ||
| return chain_cls(client_info=client_info, db_conn=db_conn) | ||
| if chain_name == "holdings_analysis": | ||
| return chain_cls(client_info=client_info, db_conn=db_conn) | ||
| return chain_cls(llm=client_info.client if client_info else None, db_conn=db_conn) |
| filters.append(f"- Restrict results to cusips={cusips}") | ||
| if not filters: | ||
| return "" | ||
| return "Context filters:\n" + "\n".join(filters) + "\n" |
- [x] format structured filing and holdings results for ChatResponse\n- [x] align fallback chain signatures with real chain contracts\n- [x] guard context-derived prompt inputs against injection\n- [x] add regression coverage for review-raised edge cases
|
Addressed the Copilot review items in 681a516.\n\nChanges made:\n- format real filing-summary and holdings-analysis model results into non-empty text\n- align local fallback chain signatures with the real chain contracts used by \n- apply prompt-injection guards to context-derived NL query and RAG inputs, not just the primary question\n- add regression tests for the structured-response formatting, fallback signatures, and context-injection paths\n\nLocal validation rerun after the patch:\n- All checks passed!\n- \n- ........................................................................ [ 82%] api/chat.py:1185 ../../../../../../../../opt/anaconda3/lib/python3.12/site-packages/fastapi/applications.py:4599 api/chat.py:1480 api/chat.py:1487 ../../../../../../../../opt/anaconda3/lib/python3.12/site-packages/huggingface_hub/file_download.py:943 :488 :488 -- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html\n- tests/test_research_ui.py:58: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] |
|
Addressed the Copilot review items in 681a516. Changes made:
Local validation rerun after the patch:
|
Summary\n- repair chat chain construction/invocation so filing summary, holdings analysis, NL query, and RAG use the real chain contracts\n- normalize request context and propagate manager/date/cusip filters into NL query and RAG flows\n- add regression coverage for adapter-level routing and Postgres-safe RAG structured queries\n\n## Validation\n- python -m ruff check api/chat.py chains/nl_query.py chains/rag_search.py tests/test_chat_api.py tests/test_nl_query_chain.py tests/test_rag_search_chain.py\n- python -m black --check api/chat.py chains/nl_query.py chains/rag_search.py tests/test_chat_api.py tests/test_nl_query_chain.py tests/test_rag_search_chain.py\n- python -m pytest -q tests/test_chat_api.py tests/test_nl_query_chain.py tests/test_rag_search_chain.py\n- python -m pytest -q tests/test_feedback.py tests/test_research_ui.py tests/test_search.py\n- python -m mypy api/chat.py chains/nl_query.py chains/rag_search.py tests/test_chat_api.py tests/test_nl_query_chain.py tests/test_rag_search_chain.py tests/test_feedback.py tests/test_research_ui.py tests/test_search.py\n