LIANJie-Jason Claude Opus 4.6 commited on
Commit
6b689a5
·
1 Parent(s): 7e9f52f

fix: SQL fuzzy matching fallback for country names with alternate forms

Browse files

Exact match (=) tried first; if 0 rows, auto-retries with LIKE '%...%'.
Match type (exact/fuzzy) transparently reported in UI source summaries.
Fixes queries like "South Korea" matching "South Korea (Republic of Korea)".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

app_cli.py CHANGED
@@ -110,7 +110,9 @@ def _format_sources(retrieval_result: dict) -> str:
110
  if sql_results:
111
  if db_results:
112
  lines.append("")
113
- lines.append(f"SQL Results: {len(sql_results)} row(s) returned")
 
 
114
 
115
  if web_results:
116
  if db_results or sql_results:
@@ -405,9 +407,11 @@ def main() -> None:
405
  db_count = len(retrieval_result.get("db_results", []))
406
  web_count = len(retrieval_result.get("web_results", []))
407
  sql_count = len(retrieval_result.get("sql_results", []))
 
408
  source_summary = f"[dim]Sources: {db_count} local"
409
  if sql_count:
410
- source_summary += f", {sql_count} SQL rows"
 
411
  if web_count:
412
  source_summary += f", {web_count} web"
413
  source_summary += " -- type /sources for details[/dim]"
 
110
  if sql_results:
111
  if db_results:
112
  lines.append("")
113
+ sql_match = retrieval_result.get("sql_match_type", "")
114
+ match_note = f" (matched via {sql_match} match)" if sql_match else ""
115
+ lines.append(f"SQL Results: {len(sql_results)} row(s) returned{match_note}")
116
 
117
  if web_results:
118
  if db_results or sql_results:
 
407
  db_count = len(retrieval_result.get("db_results", []))
408
  web_count = len(retrieval_result.get("web_results", []))
409
  sql_count = len(retrieval_result.get("sql_results", []))
410
+ sql_match = retrieval_result.get("sql_match_type", "")
411
  source_summary = f"[dim]Sources: {db_count} local"
412
  if sql_count:
413
+ match_label = f" ({sql_match} match)" if sql_match else ""
414
+ source_summary += f", {sql_count} SQL rows{match_label}"
415
  if web_count:
416
  source_summary += f", {web_count} web"
417
  source_summary += " -- type /sources for details[/dim]"
app_web.py CHANGED
@@ -228,9 +228,11 @@ def render_chat():
228
  n_local = len(retrieval_result.get("db_results", []))
229
  n_web = len(retrieval_result.get("web_results", []))
230
  n_sql = len(retrieval_result.get("sql_results", []))
 
231
  source_label = f"Found {n_local} local"
232
  if n_sql:
233
- source_label += f" + {n_sql} SQL rows"
 
234
  source_label += f" + {n_web} web sources. Generating response..."
235
  status.update(label=source_label)
236
 
 
228
  n_local = len(retrieval_result.get("db_results", []))
229
  n_web = len(retrieval_result.get("web_results", []))
230
  n_sql = len(retrieval_result.get("sql_results", []))
231
+ sql_match = retrieval_result.get("sql_match_type", "")
232
  source_label = f"Found {n_local} local"
233
  if n_sql:
234
+ match_label = f" ({sql_match} match)" if sql_match else ""
235
+ source_label += f" + {n_sql} SQL rows{match_label}"
236
  source_label += f" + {n_web} web sources. Generating response..."
237
  status.update(label=source_label)
238
 
docs/BUGFIX_LOG.md ADDED
@@ -0,0 +1,288 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ # Bug Fix Log — RAG Research Chatbot Template
2
+
3
+ Tracks all bugs found and fixed across audit sessions. Each entry includes the
4
+ severity, file, root cause, fix applied, and the audit round that found it.
5
+
6
+ ---
7
+
8
+ ## Legend
9
+
10
+ - **Severity**: CRITICAL (crash/security) / HIGH (incorrect behavior) / MEDIUM (edge case/robustness) / LOW (cosmetic/minor)
11
+ - **Status**: FIXED / KNOWN (documented, not yet fixed) / WONTFIX (acceptable trade-off)
12
+ - **Round**: Which audit session found and fixed the bug
13
+
14
+ ---
15
+
16
+ ## Round 1 — 2026-03-05 (Initial Audit)
17
+
18
+ | # | Severity | File | Bug | Fix | Status |
19
+ |---|----------|------|-----|-----|--------|
20
+ | 1 | CRITICAL | `setup.py` | `generate_config()` produced wrong config field names (`vector_store` instead of `vector_db`, `provider` instead of `backend`, etc.) | Corrected all field names to match codebase consumers | FIXED |
21
+ | 2 | HIGH | `setup.py:165` | Wrong run command (`python -m src.cli` instead of `python app_cli.py`) | Corrected command string | FIXED |
22
+ | 3 | MEDIUM | `app_cli.py:207` | Dead code branch (`if False` ternary in input prompt) | Removed dead code | FIXED |
23
+
24
+ ---
25
+
26
+ ## Round 2 — 2026-03-05 (6-Agent Audit)
27
+
28
+ | # | Severity | File | Bug | Fix | Status |
29
+ |---|----------|------|-----|-----|--------|
30
+ | 4 | HIGH | `src/retriever.py` | `build_combined_context()` hardcoded `knowledge_base/` prefix in source paths | Made prefix dynamic from config `paths.knowledge_base` | FIXED |
31
+ | 5 | HIGH | `src/verifier.py` | `parse_verification_result()` crashed on malformed JSON with no fallback | Added `raw_decode` fallback for JSON extraction from LLM text | FIXED |
32
+ | 6 | HIGH | `src/prompts.py` | System prompt used `[N]` placeholder which LLMs copied literally | Changed to "numbered endnote like [1], [2]" with explicit "Do NOT output [N]" | FIXED |
33
+ | 7 | MEDIUM | `src/retriever.py` | Fixed top_k retrieval returning too many irrelevant chunks | Added `max_distance` threshold filter (default 0.55) | FIXED |
34
+ | 8 | MEDIUM | `src/verifier.py` | `compute_soft_max_tokens()` could return 0 for very short contexts | Added `max(256, ...)` floor | FIXED |
35
+ | 9 | MEDIUM | `src/ingest.py` | Old ChromaDB data persisted across re-ingestion | Added deferred clearing (clear old data only after first successful file read) | FIXED |
36
+ | 10 | MEDIUM | `src/retriever.py` | No fallback when vector search returns zero results | Added meta chunk fallback from KB overview | FIXED |
37
+ | 11 | LOW | `src/prompts.py` | Verification prompt missing SQL results priority check | Added check #6: "Is the source priority respected (local documents = SQL results > web sources)?" | FIXED |
38
+
39
+ ---
40
+
41
+ ## Round 3 — 2026-03-05 (4-Agent Focused Audit)
42
+
43
+ | # | Severity | File | Bug | Fix | Status |
44
+ |---|----------|------|-----|-----|--------|
45
+ | 12 | HIGH | `src/sql_retriever.py` | SQL injection: no SELECT-only validation | Added `_validate_sql()` with SELECT-only check | FIXED |
46
+ | 13 | HIGH | `src/sql_retriever.py` | SQL injection: no dangerous keyword blocklist | Added `_DANGEROUS_KEYWORDS` regex (INSERT, UPDATE, DELETE, DROP, ALTER, CREATE, ATTACH, DETACH, PRAGMA, LOAD_EXTENSION) | FIXED |
47
+ | 14 | MEDIUM | `src/sql_retriever.py` | SQL injection: no read-only connection | Added `?mode=ro` to SQLite URI | FIXED |
48
+ | 15 | MEDIUM | `src/sql_retriever.py` | Semicolons not blocked in SQL queries | Added semicolon rejection in `_validate_sql()` | FIXED |
49
+ | 16 | MEDIUM | `src/retriever.py` | Chunk IDs not prefixed by type (local vs SQL vs web) | Added `CHUNK-LOCAL-NNN`, `CHUNK-SQL-NNN`, `CHUNK-WEB-NNN` prefixes | FIXED |
50
+ | 17 | MEDIUM | `src/prompts.py` | System prompt referenced chunk IDs that shouldn't appear in user-facing output | Added "Do NOT use chunk IDs in the reference list" instruction | FIXED |
51
+ | 18 | MEDIUM | `src/verifier.py` | Layer 4 used embedding-based similarity (expensive) | Replaced with term-overlap semantic similarity (zero API cost) | FIXED |
52
+ | 19 | MEDIUM | `src/sql_retriever.py` | `format_sql_results_as_context()` didn't include table source file info | Added `table_info` parameter with source file and column descriptions | FIXED |
53
+ | 20 | MEDIUM | `app_cli.py` | No transparency message for reformulated queries | Added "Searching for: ..." display when query is reformulated | FIXED |
54
+ | 21 | MEDIUM | `src/retriever.py` | SQL-to-vector fallback not implemented | Added fallback: if SQL returns empty and route was "sql", try vector search | FIXED |
55
+
56
+ ---
57
+
58
+ ## Round 4 — 2026-03-05 (4-Agent Workflow-Tracing Audit)
59
+
60
+ | # | Severity | File | Bug | Fix | Status |
61
+ |---|----------|------|-----|-----|--------|
62
+ | 22 | HIGH | `src/query_engine.py` | `understand_query()` crashed when `sql.enabled` was true but no schema existed | Added guard: only load schema when `sql_db/sql_schemas.json` exists | FIXED |
63
+ | 23 | HIGH | `src/retriever.py` | `retrieve()` didn't pass `sql_query` from QU result to SQL retriever | Added `sql_query` parameter threading through retrieve pipeline | FIXED |
64
+ | 24 | HIGH | `src/verifier.py` | `verify_and_respond()` didn't use `display_query` for the LLM question | Changed to accept and use `display_query` (clear question) vs `search_query` (keyword-optimized) | FIXED |
65
+ | 25 | HIGH | `app_cli.py`, `app_web.py` | Apps didn't pass `display_query` to `verify_and_respond()` | Updated both UIs to pass `display_query` | FIXED |
66
+
67
+ ---
68
+
69
+ ## Round 5 — 2026-03-05 (6 MEDIUM Bugs)
70
+
71
+ | # | Severity | File | Bug | Fix | Status |
72
+ |---|----------|------|-----|-----|--------|
73
+ | 26 | MEDIUM | `src/prompts.py` | `str.format()` on templates with curly braces in user content caused KeyError | Verified safe by `str.format()` semantics (substituted values not re-parsed); added regression tests | FIXED |
74
+ | 27 | MEDIUM | `src/prompts.py` | QU prompt JSON example braces `{{}}` could confuse LLM | Verified correct — `{{}}` in template produces literal `{}` in output | FIXED |
75
+ | 28 | MEDIUM | `src/sql_ingest.py` | Column name deduplication didn't handle pre-existing `_2` suffixes | Added collision-aware suffix generation | FIXED |
76
+ | 29 | MEDIUM | `src/sql_retriever.py` | `execute_sql_query()` didn't strip markdown code fences from LLM-generated SQL | Added fence stripping before validation | FIXED |
77
+ | 30 | MEDIUM | `src/sql_retriever.py` | `_lookup_source_file()` was private but imported by `retriever.py` | Made it a public function (removed underscore prefix) | FIXED |
78
+ | 31 | MEDIUM | `src/retriever.py` | `_extract_table_from_query()` didn't handle backtick-quoted table names | Extended regex to match backtick quoting | FIXED |
79
+
80
+ ---
81
+
82
+ ## Round 6 — 2026-03-06 Session 1 (KB Meta Enrichment)
83
+
84
+ | # | Severity | File | Bug | Fix | Status |
85
+ |---|----------|------|-----|-----|--------|
86
+ | 32 | MEDIUM | `src/kb_meta.py` | `generate_kb_overview_with_llm()` didn't pass enriched schema (column descriptions, samples, stats) to LLM prompt | Expanded SQL section with full column detail: type, unique count, min/max, samples, descriptions, codebook attribution | FIXED |
87
+ | 33 | MEDIUM | `src/kb_meta.py` | Deterministic fallback `generate_kb_overview()` didn't include column descriptions or stats | Added table descriptions, column descriptions, stats, and samples to deterministic output | FIXED |
88
+ | 34 | MEDIUM | `src/prompts.py` | QU prompt didn't instruct LLM to use KB overview for query standardization | Added SEARCH rule: "use exact dataset names, column names, file names from KB overview" | FIXED |
89
+ | 35 | MEDIUM | `src/prompts.py` | QU prompt didn't instruct LLM to use KB overview for informed clarification | Added CLARIFY rule: "reference specific datasets, columns, topics from KB overview as options" | FIXED |
90
+
91
+ ---
92
+
93
+ ## Round 7 — 2026-03-06 Session 2 (12-Agent Deep Audit)
94
+
95
+ ### CRITICAL Fixes
96
+
97
+ | # | Severity | File | Bug | Fix | Status |
98
+ |---|----------|------|-----|-----|--------|
99
+ | 36 | CRITICAL | `src/sql_retriever.py:10` | SQL injection: `UNION SELECT` not blocked — allowed schema exfiltration via `SELECT * FROM t UNION SELECT * FROM sqlite_master` | Added `UNION` to `_DANGEROUS_KEYWORDS` regex | FIXED |
100
+ | 37 | CRITICAL | `src/ingest.py:225` | No try/except around `collection.add()` — embedding API failure (network error, rate limit) crashed entire ingestion run, losing progress for all remaining files | Wrapped in try/except; only counts successfully embedded chunks; prints warning and continues | FIXED |
101
+ | 38 | CRITICAL | `src/ingest.py:221-222` | Chunk ID collision — files with paths differing only in `/` vs `_` (e.g., `subdir/data.csv` and `subdir_data.csv`) produced identical IDs, causing ChromaDB add() error | Replaced path-based prefix with `hashlib.md5(source_name.encode()).hexdigest()[:12]` | FIXED |
102
+
103
+ ### HIGH Fixes
104
+
105
+ | # | Severity | File | Bug | Fix | Status |
106
+ |---|----------|------|-----|-----|--------|
107
+ | 39 | HIGH | `src/retriever.py:204` | Vector-to-SQL fallback was dead code — required `sql_query` to be non-None, but `sql_query` is always None when `route="vector"` | Added `_build_fallback_sql_query()` that builds a LIKE-based keyword search from schema; removed `sql_query` guard from fallback condition | FIXED |
108
+ | 40 | HIGH | `src/verifier.py:242` | Blocked-response detection false positive — `response.startswith("[") and "blocked" in response.lower()` matched legitimate responses like `[1] The road was blocked by protesters...` | Changed to case-insensitive check for specific prefixes: `lower_resp.startswith("[gemini blocked") or lower_resp.startswith("[blocked")` | FIXED |
109
+ | 41 | HIGH | `src/verifier.py:276-341` | Last-iteration correction returned unverified — in non-strict mode, the 3rd correction was never verified before being sent to user | Added post-loop verification: if response differs from initial, run one final verification pass before exhausted-iterations handling | FIXED |
110
+ | 42 | HIGH | `src/llm/openai.py:11-15` | OpenAI o-series reasoning models (o1/o3/o4) incompatible — `temperature`, `max_tokens`, and `system` role all rejected by these models, but `list_models()` included them | Added `is_reasoning` detection; uses `developer` role, `max_completion_tokens`, omits `temperature` for o-series | FIXED |
111
+ | 43 | HIGH | `setup.py:96-102` | `.env` writer didn't quote values — API keys containing `#` silently truncated (treated as comments by dotenv parsers) | Wrapped values in double quotes: `KEY="value"`; updated parser to strip quotes when reading | FIXED |
112
+ | 44 | HIGH | `setup.py:134-148` | Setup wizard silently fell back to hardcoded model list when API key was invalid — no warning to user that key validation failed | Added warning message: "Could not validate API key for {provider}. Using default model list." | FIXED |
113
+
114
+ ### MEDIUM Fixes
115
+
116
+ | # | Severity | File | Bug | Fix | Status |
117
+ |---|----------|------|-----|-----|--------|
118
+ | 45 | MEDIUM | `src/sql_retriever.py:15-31` | SQL comments (`--` and `/* */`) not stripped before validation — could hide payloads | Added `_strip_sql_comments()` function; called before all validation checks | FIXED |
119
+ | 46 | MEDIUM | `src/sql_retriever.py:40-42` | Subqueries not restricted despite docstring claiming they were — `SELECT * FROM (SELECT * FROM sqlite_master)` passed validation | Added check: reject queries with more than one `SELECT` keyword | FIXED |
120
+ | 47 | MEDIUM | `src/query_engine.py:164-171` | No type validation on LLM-returned `search_query`, `display_query`, `sql_query` — non-string types (list, dict, int) crashed downstream | Added `isinstance(x, str)` checks with fallback to `original_query` or `None` | FIXED |
121
+ | 48 | MEDIUM | `src/verifier.py:169,181` | `parse_verification_result()` didn't validate `errors`/`error_count` types — malformed LLM output caused downstream crashes | Added type validation: `errors` must be list (default `[]`), `error_count` must be int (default `len(errors)`) | FIXED |
122
+ | 49 | MEDIUM | `src/config_loader.py:33-37` | YAML null sections crashed all chained `.get()` patterns — `cfg.get("llm", {})` returns `None` (not `{}`) when key exists with value `None` | Added normalization loop after `yaml.safe_load()`: replaces `None`-valued top-level keys with empty dicts | FIXED |
123
+ | 50 | MEDIUM | `src/verifier.py:247` | Empty LLM response (`""`) passed all verification layers and was returned to user as blank | Added empty-response guard: returns `NO_SOURCES_REFUSAL` with `refused=True` if response is empty/whitespace | FIXED |
124
+ | 51 | MEDIUM | `src/sql_ingest.py:558`, `src/sql_ingest.py:43` | `pd.NA` (`"<NA>"`), `pd.NaT` (`"NaT"`), and other common missing indicators (N/A, null, None, .) not recognized as null — forced numeric columns to TEXT | Added `_NULL_STRINGS` set and `_is_null()` helper; updated all 4 null-checking locations | FIXED |
125
+ | 52 | MEDIUM | `src/sql_ingest.py:339-351`, `src/readers/excel.py:13-21` | openpyxl workbook not closed on exception — file handle leaked | Wrapped workbook iteration in `try/finally` blocks with `wb.close()` | FIXED |
126
+
127
+ ### LOW Fixes
128
+
129
+ | # | Severity | File | Bug | Fix | Status |
130
+ |---|----------|------|-----|-----|--------|
131
+ | 53 | LOW | `src/ingest.py:179-181` | Overlap guard only activated at 100% overlap — `chunk_overlap = chunk_size - 1` (99.9% overlap) passed through, creating enormous numbers of near-duplicate chunks | Changed guard to cap overlap at 80% of chunk_size | FIXED |
132
+ | 54 | LOW | `setup.py:136-137` | Empty API key (Enter at prompt) silently accepted and written to `.env` | Added warning: "No API key entered for {provider}. You can set it later in .env" | FIXED |
133
+
134
+ ---
135
+
136
+ ## Round 8 — 2026-03-06 Session 3 (6-Agent Strict Audit + Cross-Check)
137
+
138
+ ### CRITICAL Fixes
139
+
140
+ | # | Severity | File | Bug | Fix | Status |
141
+ |---|----------|------|-----|-----|--------|
142
+ | 55 | CRITICAL | `src/sql_retriever.py` | SQL injection: `execute_sql_query()` executed original query, not comment-stripped version — attacker could hide dangerous SQL in comments that validation strips but execution preserves | Strip comments before BOTH validation and execution: `sql_query = _strip_sql_comments(sql_query).strip()` at top of `execute_sql_query()` | FIXED |
143
+
144
+ ### HIGH Fixes
145
+
146
+ | # | Severity | File | Bug | Fix | Status |
147
+ |---|----------|------|-----|-----|--------|
148
+ | 56 | HIGH | `src/verifier.py` | `parse_verification_result(None)` crashed — `enumerate(None)` in raw_decode loop; post-loop `generate()` returning None caused unhandled crash | Added `if not raw:` guard at top returning synthetic fail result `{"pass": False, "errors": [...], "error_count": 1}` | FIXED |
149
+ | 57 | HIGH | `src/verifier.py` | Correction `generate()` returning None → `None.lower()` crash on next iteration; also unhandled exceptions from any `generate()` in verification loop | Wrapped ALL verification/correction `generate()` calls in try/except; added `if not response or not response.strip():` guard falling back to `initial_response` | FIXED |
150
+ | 58 | HIGH | `src/sql_retriever.py` | Comment stripping not iterative — nested comments (`/* /* hidden */ DROP TABLE */`) could bypass single-pass stripping | Changed to `while prev != sql` loop that iterates until stable | FIXED |
151
+ | 59 | HIGH | `src/sql_retriever.py` | `_DANGEROUS_KEYWORDS` missing 12 defense-in-depth entries — REPLACE, VACUUM, REINDEX, SAVEPOINT, RELEASE, ROLLBACK, BEGIN, COMMIT, GRANT, REVOKE, EXPLAIN, WITH all unblocked | Expanded regex from 11 to 23 keywords | FIXED |
152
+ | 60 | HIGH | `src/ingest.py` | Data loss if all `collection.add()` calls fail after deferred clear — clear happened before first successful add, so if embedding API was down, all existing data was wiped with nothing to replace it | Moved deferred clear logic INSIDE the try block, only triggers on first SUCCESSFUL `collection.add()` | FIXED |
153
+ | 61 | HIGH | `src/readers/excel.py` | `_rows_to_pages([])` crashed with IndexError — `rows[0]` on empty list when openpyxl returns empty sheet | Added `if not rows: return []` guard at top | FIXED |
154
+
155
+ ### MEDIUM Fixes
156
+
157
+ | # | Severity | File | Bug | Fix | Status |
158
+ |---|----------|------|-----|-----|--------|
159
+ | 62 | MEDIUM | `src/verifier.py` | `"pass": "false"` (string) was truthy, incorrectly passing verification — LLM could return string instead of boolean | Added `result["pass"] = result.get("pass") is True` strict boolean normalization in BOTH parsing paths | FIXED |
160
+ | 63 | MEDIUM | `src/sql_ingest.py` | `"na"` missing from `_NULL_STRINGS` — R-style NA values not recognized as null, forcing numeric columns to TEXT type | Added `"na"` to `_NULL_STRINGS` set | FIXED |
161
+ | 64 | MEDIUM | `src/llm/gemini.py` | `generate_content()` only caught ValueError (safety filters) — API errors, network errors, auth errors all unhandled, crashing the pipeline | Added broad `except Exception as e` returning `f"[Gemini error: {e}]"` | FIXED |
162
+
163
+ ### LOW Fixes
164
+
165
+ | # | Severity | File | Bug | Fix | Status |
166
+ |---|----------|------|-----|-----|--------|
167
+ | 65 | LOW | `src/verifier.py` | Empty LLM response showed misleading "no information in knowledge base" when sources DID exist | Changed to distinct message: "The AI model was unable to generate a response. Please try rephrasing your question." | FIXED |
168
+ | 66 | LOW | `setup.py` | Model selection "0" indexed `models[-1]` via negative indexing (`int("0") - 1 = -1`) | Added bounds check: `if 0 <= idx < len(models)` | FIXED |
169
+ | 67 | LOW | `app_cli.py` | Missing `route`/`sql_query` keys in exception fallback dicts — `qu_result.get("route")` returned None, passed to `retrieve()` as route=None | Added `"route": "vector", "sql_query": None` to both fallback dicts | FIXED |
170
+ | 68 | LOW | `src/retriever.py` | No SQL fallback when `route="both"` and both paths empty — only triggered for `route="vector"` | Extended condition to `route in ("vector", "both")` | FIXED |
171
+ | 69 | LOW | `app_web.py:187` | QU exception fallback dict missing `"route"` and `"sql_query"` keys — inconsistent with CLI fix (non-crashing due to `.get()` defaults + pre-initialized locals) | Added `"route": "vector", "sql_query": None` for consistency | FIXED |
172
+
173
+ ---
174
+
175
+ ## Known Issues (Not Yet Fixed)
176
+
177
+ | # | Severity | File | Description | Reason |
178
+ |---|----------|------|-------------|--------|
179
+ | K1 | MEDIUM | `src/sql_ingest.py:562` | `int(float(...))` loses precision for 16+ digit integers | Rare in research datasets; fix requires careful refactoring of type conversion pipeline |
180
+ | K2 | MEDIUM | Design doc | Verification loop branching logic description doesn't match uniform loop in code | Doc update needed, not a code bug |
181
+ | K3 | MEDIUM | Design doc | 9-point verification checklist items differ between doc and code | Doc update needed, not a code bug |
182
+ | K4 | MEDIUM | `CLAUDE.md` | Config table missing 9+ fields (web_search.enabled, chatbot.name, llm.provider, etc.) | Doc update needed |
183
+ | K5 | LOW | `src/sql_retriever.py` | SQL validation false positive when data values contain "UNION" or "SELECT" (e.g., `WHERE org = 'European Union'`) | Acceptable security trade-off — overly strict is safer than exploitable |
184
+ | K10 | LOW | `src/sql_retriever.py` | No SQL execution timeout — Cartesian product queries could hang indefinitely | Would need separate thread/process; SQLite has no built-in timeout for SELECT |
185
+ | K11 | LOW | `src/retriever.py` | `_build_fallback_sql_query()` uses string interpolation instead of parameterized queries | Mitigated by `\W+` word splitting which strips SQL metacharacters |
186
+ | K12 | LOW | Multiple | `cfg.get("section", {})` systemic pattern — if YAML section exists but is `None`, returns `None` not `{}` | Partially mitigated by config_loader normalization; remaining ~30 locations are low risk |
187
+ | K6 | LOW | `src/search/semantic_scholar.py` | Filters out papers without abstracts, reducing result count for some topics | By design — abstract-less papers can't provide useful context |
188
+ | K7 | LOW | `src/retriever.py` | LIKE wildcards `_`/`%` not escaped in fallback SQL query | Minor — extra matches unlikely to cause visible problems |
189
+ | K8 | LOW | `CLAUDE.md` | Test count says 162, should be 165 | Doc update needed |
190
+ | K9 | LOW | `src/verifier.py` | `compute_similarity_flags` regex `[a-z]{3,}` excludes non-ASCII and 2-letter acronyms (AI, US, UK) | Advisory-only layer; impact limited to noisy flags |
191
+
192
+ ---
193
+
194
+ ## Test Coverage Gaps (Identified Round 7)
195
+
196
+ | Priority | What | Description |
197
+ |----------|------|-------------|
198
+ | CRITICAL | `retrieve()` | Central routing/fallback orchestration — zero direct tests |
199
+ | CRITICAL | `verify_and_respond()` loop | Iterative correction — zero tests for multi-iteration scenarios |
200
+ | CRITICAL | `compute_similarity_flags()` | Layer 4 — entire function untested |
201
+ | HIGH | `_build_fallback_sql_query()` | New fallback SQL builder — zero tests |
202
+ | HIGH | `app_web.py` | Entire Streamlit web UI — zero test file |
203
+ | HIGH | Clarification flow | CLI/web clarification loop — untested |
204
+ | HIGH | `semantic_scholar.py` | Retry logic, rate limiting, result formatting — untested |
205
+ | HIGH | SQL fallback paths | Both directions (SQL→vector, vector→SQL) — untested |
206
+
207
+ ---
208
+
209
+ ## Round 9 — 2026-03-06 Session 4 (6-Agent Final Inspection + Anti-Hallucination Audit)
210
+
211
+ This round was a comprehensive pre-release inspection using 6 parallel audit agents (pipeline integrity, security, error handling, test coverage, cross-cutting consistency, anti-hallucination stack), followed by 3 parallel fix agents and 2 parallel inspector agents.
212
+
213
+ ### CRITICAL Fixes
214
+
215
+ | # | Severity | File | Bug | Fix | Status |
216
+ |---|----------|------|-----|-----|--------|
217
+ | 70 | CRITICAL | `src/verifier.py:343-349` | **AH-06**: Correction prompt missing previous response — LLM told "your previous response failed" but never shown what failed; correction loop was blind regeneration, not targeted fixing | Included `f"Your previous response was:\n{response}\n\n"` in correction prompt | FIXED |
218
+
219
+ ### HIGH Fixes
220
+
221
+ | # | Severity | File | Bug | Fix | Status |
222
+ |---|----------|------|-----|-----|--------|
223
+ | 71 | HIGH | `src/ingest.py:160-164` | **BUG-P1**: Stale data persists after removing all files — re-ingesting with empty `knowledge_base/` returned early without clearing ChromaDB/SQLite/KB meta | Added cleanup of all 3 data stores (ChromaDB chunks, SQLite DB + schema, KB meta file) before returning 0; each cleanup independently try/except guarded | FIXED |
224
+ | 72 | HIGH | `src/verifier.py` | **AH-12**: No automated citation-to-source mapping — nothing verified that `[N]` in response corresponds to correct source; LLM could misattribute every citation | Added `validate_citations()` deterministic audit (Layer 4.5): checks citation numbers don't exceed source count, verifies References section mentions actual retrieved filenames; integrated into verification loop and non-strict warning | FIXED |
225
+ | 73 | HIGH | `src/prompts.py` | **AH-07**: Self-verification bias — verification prompt's checklist item 2 too vague ("Are all citations grounded?") | Strengthened to require EXACT evidence identification; added checklist item 10 for citation number/source cross-validation; upgraded to 10-point checklist | FIXED |
226
+ | 74 | HIGH | `src/verifier.py:283-303` | **AH-08**: `max_iterations=0` or `verification.enabled=false` silently disabled ALL anti-hallucination verification with no warning | Added `print("WARNING: ...")` for both disabled paths so users/operators know verification is bypassed | FIXED |
227
+ | 75 | HIGH | `src/llm/gemini.py:17-22` | **ERR-1**: `NameError` crash when `generate_content()` raises `ValueError` before `response` assigned — except handler referenced unbound `response` variable | Added `response = None` initialization before try; added `if response is not None:` guard in ValueError handler | FIXED |
228
+ | 76 | HIGH | `src/llm/openai.py`, `src/llm/anthropic.py` | **ERR-2**: Zero error handling in `generate()` — raw SDK exceptions (AuthenticationError, RateLimitError, APIConnectionError) propagated as unhelpful tracebacks | Wrapped API calls in try/except; re-raise as `RuntimeError` with sanitized message (error type only, no raw exception text to prevent API key leaks) | FIXED |
229
+
230
+ ### MEDIUM Fixes
231
+
232
+ | # | Severity | File | Bug | Fix | Status |
233
+ |---|----------|------|-----|-----|--------|
234
+ | 77 | MEDIUM | `src/llm/gemini.py:23-24` | **SEC-1**: Raw exception message in `f"[Gemini error: {e}]"` could leak API key fragments or internal URLs to user | Changed to static `"[Gemini error: request failed]"` — no raw exception text | FIXED |
235
+ | 78 | MEDIUM | `.gitignore` | **SEC-2**: `config.yaml` not gitignored despite CLAUDE.md documenting it as gitignored; setup wizard generates `api_keys:` section inviting users to put keys there | Added `config.yaml` to `.gitignore` | FIXED |
236
+ | 79 | MEDIUM | `src/verifier.py:272` | Gemini error strings `"[Gemini error: ...]"` not caught by blocked-response detector — leaked into verification loop, wasting 6+ LLM calls | Added `lower_resp.startswith("[gemini error")` to the guard condition | FIXED |
237
+ | 80 | MEDIUM | `src/verifier.py:380` | **INSPECT-1**: `citation_warnings` computed once from initial response, never recomputed after corrections — stale warnings fed to verifier on subsequent iterations | Moved `validate_citations()` call inside the loop so it recomputes each iteration | FIXED |
238
+
239
+ ### Known Issues Added
240
+
241
+ | # | Severity | File | Description | Reason |
242
+ |---|----------|------|-------------|--------|
243
+ | K13 | HIGH | `src/llm/gemini.py` + `requirements.txt` | **CON-4**: `google-generativeai` package fully deprecated; replacement `google-genai` has different API surface | Requires full API migration; functional for now but will break when PyPI removes package |
244
+ | K14 | MEDIUM | `src/verifier.py` | **AH-05**: Token cap step function too generous for small contexts (501 chars → 1536 tokens = 12:1 ratio) | Would benefit from continuous formula; current 3-tier approach is functional |
245
+ | K15 | MEDIUM | `src/verifier.py:24-33` | **AH-11**: Warning phrase list missing common patterns: "studies have shown", "research suggests", "historically" | Advisory layer only; expandable post-release |
246
+ | K16 | LOW | `src/verifier.py:184` | **INSPECT-2**: References section regex matches "sources" in prose, not just section header | Mitigated: filename matching still works because match includes everything to end of response |
247
+ | K17 | LOW | `tests/test_ingest.py` | **INSPECT-5**: No test coverage for new empty-dir cleanup code path | Should add test for empty knowledge_base/ clearing stale data |
248
+
249
+ ---
250
+
251
+ ## Round 10 — 2026-03-06 Session 5 (SQL Fuzzy Matching)
252
+
253
+ User-reported bug: querying "south korea in pts" returned zero SQL results despite PTS dataset being ingested, because country names in the dataset use parenthetical forms (e.g., `"South Korea (Republic of Korea)"`). The LLM-generated SQL used exact `= 'South Korea'` which missed the match.
254
+
255
+ ### HIGH Fixes
256
+
257
+ | # | Severity | File | Bug | Fix | Status |
258
+ |---|----------|------|-----|-----|--------|
259
+ | 81 | HIGH | `src/sql_retriever.py`, `src/retriever.py` | SQL exact match fails for country names with alternate forms (e.g., "South Korea (Republic of Korea)", "Iran (Islamic Republic of)") — LLM generates `= 'South Korea'` which returns 0 rows | Added `make_fuzzy_query()` that converts `= 'value'` → `LIKE '%value%'` for quoted string values; `_run_sql_retrieval()` now tries exact match first, auto-retries with fuzzy if 0 rows; returns `match_type` ("exact"/"fuzzy") for transparent reporting | FIXED |
260
+ | 82 | HIGH | `app_cli.py`, `app_web.py` | No transparency about which SQL matching strategy produced results — user couldn't tell if exact or fuzzy match was used | Source summary now shows `(exact match)` or `(fuzzy match)` label; `/sources` command shows `(matched via exact/fuzzy match)` | FIXED |
261
+
262
+ ### MEDIUM Fixes
263
+
264
+ | # | Severity | File | Bug | Fix | Status |
265
+ |---|----------|------|-----|-----|--------|
266
+ | 83 | MEDIUM | `src/prompts.py` | QU prompt had no guidance on SQL text matching strategy | Added instruction: use exact `=` for text columns; system handles fuzzy fallback automatically | FIXED |
267
+
268
+ ### Files Changed
269
+
270
+ - `src/sql_retriever.py` — Added `make_fuzzy_query()` function
271
+ - `src/retriever.py` — `_run_sql_retrieval()` returns 3-tuple with `match_type`; all callers updated
272
+ - `src/prompts.py` — SQL routing instructions updated
273
+ - `app_cli.py` — Source summary and `/sources` show match type
274
+ - `app_web.py` — Source label shows match type
275
+ - `tests/test_retriever.py` — Updated 5 mock return values from 2-tuple to 3-tuple
276
+
277
+ ---
278
+
279
+ ## Statistics
280
+
281
+ | Metric | Count |
282
+ |--------|-------|
283
+ | Total bugs fixed | ~119 (3 this round) |
284
+ | Audit rounds completed | 10 (10 sessions, 30+ total agent dispatches) |
285
+ | Test count | 174 passing |
286
+ | Test files | 17 |
287
+ | Source files audited | All 20+ |
288
+ | Known issues remaining | 17 (0 CRITICAL, 1 HIGH, 6 MEDIUM, 10 LOW) |
src/prompts.py CHANGED
@@ -206,6 +206,8 @@ set route to "sql" or "both".
206
  When route is "sql" or "both", you MUST provide a valid SQLite SELECT query
207
  in the "sql_query" field. When route is "vector", you SHOULD still include
208
  a "sql_query" if the data tables might answer the question as a fallback.
 
 
209
  Use only table/column names from the schema below.
210
 
211
  {schema_text}
 
206
  When route is "sql" or "both", you MUST provide a valid SQLite SELECT query
207
  in the "sql_query" field. When route is "vector", you SHOULD still include
208
  a "sql_query" if the data tables might answer the question as a fallback.
209
+ Use exact equality (=) for text columns. The system will automatically
210
+ retry with fuzzy matching (LIKE) if the exact query returns no results.
211
  Use only table/column names from the schema below.
212
 
213
  {schema_text}
src/retriever.py CHANGED
@@ -202,19 +202,40 @@ def _build_fallback_sql_query(query: str, cfg: dict) -> str | None:
202
  return None
203
 
204
 
205
- def _run_sql_retrieval(sql_query: str, cfg: dict) -> tuple[list[dict], str]:
206
  """Execute SQL query and format results as context.
207
 
208
- Returns (sql_rows, sql_context).
 
 
 
 
209
  """
210
  import json as _json
211
  import os
212
- from src.sql_retriever import execute_sql_query, format_sql_results_as_context, _lookup_source_file
 
 
 
213
 
214
  max_rows = cfg.get("sql", {}).get("max_rows", 200)
 
 
215
  sql_rows = execute_sql_query(sql_query, cfg)
 
 
 
 
 
 
 
 
 
 
 
 
216
  if not sql_rows:
217
- return [], ""
218
 
219
  sql_db_dir = cfg.get("paths", {}).get("sql_db", "sql_db")
220
  if not os.path.isabs(sql_db_dir):
@@ -229,14 +250,14 @@ def _run_sql_retrieval(sql_query: str, cfg: dict) -> tuple[list[dict], str]:
229
  schema = _json.load(f)
230
  except Exception:
231
  pass
232
- table_name = _extract_table_from_query(sql_query)
233
  source_file = _lookup_source_file(table_name, schema)
234
  table_info = schema.get(table_name)
235
  sql_context = format_sql_results_as_context(
236
- sql_rows, sql_query, source_file, table_info=table_info,
237
  max_rows=max_rows,
238
  )
239
- return sql_rows, sql_context
240
 
241
 
242
  def retrieve(
@@ -249,10 +270,11 @@ def retrieve(
249
  sql_enabled = cfg.get("sql", {}).get("enabled", True)
250
  sql_context = ""
251
  sql_rows = []
 
252
 
253
  # ── SQL retrieval ────────────────────────────────────────────────
254
  if sql_enabled and route in ("sql", "both") and sql_query:
255
- sql_rows, sql_context = _run_sql_retrieval(sql_query, cfg)
256
 
257
  # ── Vector retrieval ─────────────────────────────────────────────
258
  db_results = []
@@ -275,13 +297,13 @@ def retrieve(
275
  # LLM-generated sql_query already failed; build a fresh keyword query
276
  fallback_query = _build_fallback_sql_query(query, cfg)
277
  if fallback_query:
278
- sql_rows, sql_context = _run_sql_retrieval(fallback_query, cfg)
279
  else:
280
  # route is "vector" or "both" — existing fallback logic
281
  if not sql_query:
282
  sql_query = _build_fallback_sql_query(query, cfg)
283
  if sql_query:
284
- sql_rows, sql_context = _run_sql_retrieval(sql_query, cfg)
285
 
286
  # ── Web search ───────────────────────────────────────────────────
287
  web_enabled = cfg.get("web_search", {}).get("enabled", False)
@@ -301,5 +323,6 @@ def retrieve(
301
  "db_results": db_results,
302
  "web_results": web_results,
303
  "sql_results": sql_rows,
 
304
  "has_sources": has_sources,
305
  }
 
202
  return None
203
 
204
 
205
+ def _run_sql_retrieval(sql_query: str, cfg: dict) -> tuple[list[dict], str, str]:
206
  """Execute SQL query and format results as context.
207
 
208
+ Tries the exact query first. If it returns zero rows, automatically
209
+ retries with fuzzy matching (``=`` → ``LIKE '%…%'``).
210
+
211
+ Returns (sql_rows, sql_context, match_type) where match_type is
212
+ ``"exact"``, ``"fuzzy"``, or ``""`` (no results).
213
  """
214
  import json as _json
215
  import os
216
+ from src.sql_retriever import (
217
+ execute_sql_query, format_sql_results_as_context,
218
+ _lookup_source_file, make_fuzzy_query,
219
+ )
220
 
221
  max_rows = cfg.get("sql", {}).get("max_rows", 200)
222
+
223
+ # Step 1: try exact query
224
  sql_rows = execute_sql_query(sql_query, cfg)
225
+ effective_query = sql_query
226
+ match_type = "exact"
227
+
228
+ # Step 2: if no results, try fuzzy fallback
229
+ if not sql_rows:
230
+ fuzzy = make_fuzzy_query(sql_query)
231
+ if fuzzy:
232
+ sql_rows = execute_sql_query(fuzzy, cfg)
233
+ if sql_rows:
234
+ effective_query = fuzzy
235
+ match_type = "fuzzy"
236
+
237
  if not sql_rows:
238
+ return [], "", ""
239
 
240
  sql_db_dir = cfg.get("paths", {}).get("sql_db", "sql_db")
241
  if not os.path.isabs(sql_db_dir):
 
250
  schema = _json.load(f)
251
  except Exception:
252
  pass
253
+ table_name = _extract_table_from_query(effective_query)
254
  source_file = _lookup_source_file(table_name, schema)
255
  table_info = schema.get(table_name)
256
  sql_context = format_sql_results_as_context(
257
+ sql_rows, effective_query, source_file, table_info=table_info,
258
  max_rows=max_rows,
259
  )
260
+ return sql_rows, sql_context, match_type
261
 
262
 
263
  def retrieve(
 
270
  sql_enabled = cfg.get("sql", {}).get("enabled", True)
271
  sql_context = ""
272
  sql_rows = []
273
+ sql_match_type = ""
274
 
275
  # ── SQL retrieval ────────────────────────────────────────────────
276
  if sql_enabled and route in ("sql", "both") and sql_query:
277
+ sql_rows, sql_context, sql_match_type = _run_sql_retrieval(sql_query, cfg)
278
 
279
  # ── Vector retrieval ─────────────────────────────────────────────
280
  db_results = []
 
297
  # LLM-generated sql_query already failed; build a fresh keyword query
298
  fallback_query = _build_fallback_sql_query(query, cfg)
299
  if fallback_query:
300
+ sql_rows, sql_context, sql_match_type = _run_sql_retrieval(fallback_query, cfg)
301
  else:
302
  # route is "vector" or "both" — existing fallback logic
303
  if not sql_query:
304
  sql_query = _build_fallback_sql_query(query, cfg)
305
  if sql_query:
306
+ sql_rows, sql_context, sql_match_type = _run_sql_retrieval(sql_query, cfg)
307
 
308
  # ── Web search ───────────────────────────────────────────────────
309
  web_enabled = cfg.get("web_search", {}).get("enabled", False)
 
323
  "db_results": db_results,
324
  "web_results": web_results,
325
  "sql_results": sql_rows,
326
+ "sql_match_type": sql_match_type,
327
  "has_sources": has_sources,
328
  }
src/sql_retriever.py CHANGED
@@ -154,6 +154,25 @@ def format_sql_results_as_context(
154
  return "\n".join(parts)
155
 
156
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
157
  def _lookup_source_file(table_name: str, schema: dict) -> str:
158
  """Look up the original source file for a SQL table name."""
159
  entry = schema.get(table_name)
 
154
  return "\n".join(parts)
155
 
156
 
157
+ def make_fuzzy_query(sql: str) -> str | None:
158
+ """Convert exact equality on text values to LIKE fuzzy matching.
159
+
160
+ Transforms ``column = 'value'`` into ``column LIKE '%value%'``
161
+ so that partial country names (e.g. "South Korea" matching
162
+ "South Korea (Republic of Korea)") can be found.
163
+
164
+ Returns the modified query, or None if no substitutions were made.
165
+ """
166
+ # Match patterns like: column = 'value' or column='value'
167
+ pattern = re.compile(
168
+ r"""(["']?\w+["']?\s*)=\s*'([^']+)'""",
169
+ )
170
+ new_sql, count = pattern.subn(r"\1LIKE '%\2%'", sql)
171
+ if count == 0:
172
+ return None
173
+ return new_sql
174
+
175
+
176
  def _lookup_source_file(table_name: str, schema: dict) -> str:
177
  """Look up the original source file for a SQL table name."""
178
  entry = schema.get(table_name)
tests/test_retriever.py ADDED
@@ -0,0 +1,247 @@
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1
+ import pytest
2
+ from unittest.mock import patch, MagicMock
3
+
4
+ # ---------------------------------------------------------------------------
5
+ # Helper: base config for retrieve() tests
6
+ # ---------------------------------------------------------------------------
7
+ _BASE_CFG = {
8
+ "sql": {"enabled": True, "max_rows": 200},
9
+ "retrieval": {"top_k": 50, "max_distance": 0.55},
10
+ "web_search": {"enabled": False},
11
+ "paths": {"sql_db": "sql_db"},
12
+ }
13
+
14
+
15
+ def test_format_db_results_as_context_empty():
16
+ from src.retriever import format_db_results_as_context
17
+ assert format_db_results_as_context([]) == ""
18
+
19
+ def test_format_db_results_as_context_with_chunks():
20
+ from src.retriever import format_db_results_as_context
21
+ chunks = [{
22
+ "text": "Test content here.",
23
+ "metadata": {"source": "Dataset1/paper.pdf", "dataset": "Dataset1", "page": "3"},
24
+ "distance": 0.1,
25
+ }]
26
+ context = format_db_results_as_context(chunks)
27
+ assert "[CHUNK-LOCAL-001]" in context
28
+ assert "paper.pdf" in context
29
+ assert "Test content here." in context
30
+ assert "PRIMARY" in context
31
+
32
+
33
+ def test_format_db_results_no_hardcoded_kb_prefix():
34
+ """M6: Path should use source metadata, not hardcoded 'knowledge_base/' prefix."""
35
+ from src.retriever import format_db_results_as_context
36
+ chunks = [{
37
+ "text": "Content",
38
+ "metadata": {"source": "MyData/report.pdf", "dataset": "MyData", "page": "1"},
39
+ "distance": 0.1,
40
+ }]
41
+ context = format_db_results_as_context(chunks)
42
+ assert "MyData/report.pdf" in context
43
+ assert "knowledge_base/MyData/report.pdf" not in context
44
+
45
+ def test_build_combined_context_no_sources():
46
+ from src.retriever import build_combined_context
47
+ result = build_combined_context([], [])
48
+ assert "No relevant" in result or "don't have" in result
49
+
50
+ def test_build_combined_context_local_only():
51
+ from src.retriever import build_combined_context
52
+ db = [{"text": "data", "metadata": {"source": "f.pdf", "dataset": "d", "page": "1"}, "distance": 0.1}]
53
+ result = build_combined_context(db, [])
54
+ assert "ONLY" in result or "local" in result.lower()
55
+
56
+ def test_build_combined_context_both():
57
+ from src.retriever import build_combined_context
58
+ db = [{"text": "data", "metadata": {"source": "f.pdf", "dataset": "d", "page": "1"}, "distance": 0.1}]
59
+ web = [{"title": "P", "authors": "A", "year": 2020, "abstract": "abs", "url": "http://x", "citation_count": 1, "source_type": "web_search"}]
60
+ result = build_combined_context(db, web)
61
+ assert "PRIMARY" in result
62
+ assert "SUPPLEMENTARY" in result
63
+
64
+
65
+ def test_build_combined_context_with_sql():
66
+ from src.retriever import build_combined_context
67
+ sql = "=== SQL Query Results (PRIMARY) ===\n[CHUNK-SQL-001] x = 1"
68
+ db = [{"text": "data", "metadata": {"source": "f.pdf", "dataset": "d", "page": "1"}, "distance": 0.1}]
69
+ result = build_combined_context(db, [], sql_context=sql)
70
+ assert "SQL Query Results" in result
71
+ assert "CHUNK-LOCAL" in result
72
+
73
+
74
+ def test_build_combined_context_sql_only():
75
+ from src.retriever import build_combined_context
76
+ sql = "=== SQL Query Results (PRIMARY) ===\n[CHUNK-SQL-001] x = 1"
77
+ result = build_combined_context([], [], sql_context=sql)
78
+ assert "SQL Query Results" in result
79
+
80
+
81
+ def test_build_combined_context_sql_no_sources():
82
+ from src.retriever import build_combined_context
83
+ result = build_combined_context([], [], sql_context="")
84
+ assert "No relevant" in result or "don't have" in result
85
+
86
+
87
+ def test_retrieve_from_vectordb_fallback_to_meta():
88
+ """When no chunks pass distance filter, KB meta overview is returned."""
89
+ import chromadb
90
+ from src.kb_meta import META_CHUNK_ID, META_SOURCE, META_DATASET
91
+
92
+ client = chromadb.Client()
93
+ collection = client.get_or_create_collection("test_meta_fallback")
94
+
95
+ # Add only the meta overview chunk
96
+ collection.add(
97
+ ids=[META_CHUNK_ID],
98
+ documents=["This KB contains PTS data and Amnesty reports."],
99
+ metadatas=[{"source": META_SOURCE, "dataset": META_DATASET, "page": "1", "chunk_index": 0}],
100
+ )
101
+
102
+ # Patch get_chroma_collection to return our test collection
103
+ import src.retriever as retriever_mod
104
+ import src.ingest as ingest_mod
105
+ original = ingest_mod.get_chroma_collection
106
+
107
+ try:
108
+ ingest_mod.get_chroma_collection = lambda cfg: collection
109
+ # Use a query that won't match well + strict distance threshold
110
+ cfg = {"retrieval": {"top_k": 50, "max_distance": 0.01}}
111
+ chunks = retriever_mod.retrieve_from_vectordb("xyzzy gibberish query", cfg)
112
+ # Meta chunk should be included as fallback
113
+ assert len(chunks) == 1
114
+ assert chunks[0]["metadata"]["source"] == META_SOURCE
115
+ finally:
116
+ ingest_mod.get_chroma_collection = original
117
+ client.delete_collection("test_meta_fallback")
118
+
119
+
120
+ # ---------------------------------------------------------------------------
121
+ # Tests for retrieve() SQL fallback logic
122
+ # ---------------------------------------------------------------------------
123
+
124
+ @patch("src.retriever.search", return_value=[])
125
+ @patch("src.retriever.retrieve_from_vectordb", return_value=[])
126
+ @patch("src.retriever._run_sql_retrieval")
127
+ @patch("src.retriever._build_fallback_sql_query")
128
+ def test_retrieve_sql_route_llm_query_fails_keyword_fallback_runs(
129
+ mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
130
+ ):
131
+ """When route='sql' and LLM sql_query returns nothing, keyword fallback SQL runs."""
132
+ from src.retriever import retrieve
133
+
134
+ # First call: LLM sql_query fails (returns empty)
135
+ # Second call: keyword fallback succeeds
136
+ mock_run_sql.side_effect = [
137
+ ([], "", ""), # LLM-generated query fails
138
+ ([{"col": "val"}], "=== SQL Query Results ===\n[CHUNK-SQL-001] col = val", "fuzzy"),
139
+ ]
140
+ mock_build_fallback.return_value = 'SELECT * FROM "t" WHERE "c" LIKE \'%test%\' LIMIT 200'
141
+
142
+ result = retrieve("test data query", _BASE_CFG, route="sql", sql_query="SELECT * FROM t WHERE id=99")
143
+
144
+ # Keyword fallback should have been called
145
+ mock_build_fallback.assert_called_once_with("test data query", _BASE_CFG)
146
+ # _run_sql_retrieval called twice: once for LLM query, once for fallback
147
+ assert mock_run_sql.call_count == 2
148
+ assert result["has_sources"] is True
149
+ assert result["sql_results"] == [{"col": "val"}]
150
+
151
+
152
+ @patch("src.retriever.search", return_value=[])
153
+ @patch("src.retriever.retrieve_from_vectordb", return_value=[])
154
+ @patch("src.retriever._run_sql_retrieval")
155
+ @patch("src.retriever._build_fallback_sql_query")
156
+ def test_retrieve_sql_route_llm_query_succeeds_no_fallback(
157
+ mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
158
+ ):
159
+ """When route='sql' and LLM sql_query returns results, no fallback needed."""
160
+ from src.retriever import retrieve
161
+
162
+ mock_run_sql.return_value = (
163
+ [{"country": "CN", "score": 5}],
164
+ "=== SQL Query Results ===\n[CHUNK-SQL-001] country = CN, score = 5",
165
+ "exact",
166
+ )
167
+
168
+ result = retrieve("China PTS score", _BASE_CFG, route="sql", sql_query="SELECT * FROM pts WHERE country='CN'")
169
+
170
+ # No fallback should be attempted
171
+ mock_build_fallback.assert_not_called()
172
+ # SQL ran once (LLM query succeeded)
173
+ assert mock_run_sql.call_count == 1
174
+ # Vector should NOT have run (route="sql" and sql_rows is non-empty)
175
+ mock_vectordb.assert_not_called()
176
+ assert result["has_sources"] is True
177
+ assert result["sql_results"] == [{"country": "CN", "score": 5}]
178
+
179
+
180
+ @patch("src.retriever.search", return_value=[])
181
+ @patch("src.retriever.retrieve_from_vectordb", return_value=[])
182
+ @patch("src.retriever._run_sql_retrieval", return_value=([], "", ""))
183
+ @patch("src.retriever._build_fallback_sql_query")
184
+ def test_retrieve_vector_route_fallback_still_works(
185
+ mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
186
+ ):
187
+ """Existing behavior: route='vector', vector empty, keyword SQL fallback runs."""
188
+ from src.retriever import retrieve
189
+
190
+ mock_build_fallback.return_value = 'SELECT * FROM "t" WHERE "c" LIKE \'%protest%\' LIMIT 200'
191
+
192
+ result = retrieve("protest data", _BASE_CFG, route="vector")
193
+
194
+ # Keyword fallback should have been built
195
+ mock_build_fallback.assert_called_once_with("protest data", _BASE_CFG)
196
+ # _run_sql_retrieval called once for the fallback (no LLM query for route="vector")
197
+ assert mock_run_sql.call_count == 1
198
+ # Vector was called (route="vector")
199
+ mock_vectordb.assert_called_once()
200
+
201
+
202
+ @patch("src.retriever.search", return_value=[])
203
+ @patch("src.retriever.retrieve_from_vectordb", return_value=[])
204
+ @patch("src.retriever._run_sql_retrieval")
205
+ @patch("src.retriever._build_fallback_sql_query")
206
+ def test_retrieve_both_route_fallback_still_works(
207
+ mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
208
+ ):
209
+ """Existing behavior: route='both', both empty, keyword SQL fallback runs."""
210
+ from src.retriever import retrieve
211
+
212
+ # First call: LLM sql_query fails; second call: keyword fallback also fails
213
+ mock_run_sql.return_value = ([], "", "")
214
+ mock_build_fallback.return_value = 'SELECT * FROM "t" WHERE "c" LIKE \'%data%\' LIMIT 200'
215
+
216
+ result = retrieve("some data", _BASE_CFG, route="both", sql_query="SELECT * FROM t WHERE x=1")
217
+
218
+ # For route="both", sql_query was already set so _build_fallback_sql_query
219
+ # should NOT be called (existing behavior: reuses the sql_query from QU)
220
+ mock_build_fallback.assert_not_called()
221
+ # _run_sql_retrieval called twice: once for LLM query, once for fallback with same query
222
+ assert mock_run_sql.call_count == 2
223
+
224
+
225
+ @patch("src.retriever.search", return_value=[])
226
+ @patch("src.retriever.retrieve_from_vectordb", return_value=[])
227
+ @patch("src.retriever._run_sql_retrieval")
228
+ @patch("src.retriever._build_fallback_sql_query")
229
+ def test_retrieve_sql_route_no_sql_query_keyword_fallback_runs(
230
+ mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
231
+ ):
232
+ """When route='sql' but no sql_query provided, keyword fallback SQL runs."""
233
+ from src.retriever import retrieve
234
+
235
+ mock_run_sql.return_value = (
236
+ [{"col": "val"}],
237
+ "=== SQL Query Results ===\n[CHUNK-SQL-001] col = val",
238
+ "fuzzy",
239
+ )
240
+ mock_build_fallback.return_value = 'SELECT * FROM "t" WHERE "c" LIKE \'%test%\' LIMIT 200'
241
+
242
+ result = retrieve("test query", _BASE_CFG, route="sql", sql_query=None)
243
+
244
+ # No LLM sql_query → keyword fallback should fire
245
+ mock_build_fallback.assert_called_once()
246
+ assert mock_run_sql.call_count == 1
247
+ assert result["has_sources"] is True