Spaces:
Sleeping
Sleeping
Commit ·
e8de214
1
Parent(s): f95cdb7
fix: remove docs/ and tests/ from tracking (gitignored, local-only)
Browse files- docs/BUGFIX_LOG.md +0 -487
- tests/test_retriever.py +0 -247
docs/BUGFIX_LOG.md
DELETED
|
@@ -1,487 +0,0 @@
|
|
| 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 said 162, actual is 174 | FIXED (Round 11 doc update) |
|
| 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 | **FIXED** — Migrated to `google-genai>=1.0.0` SDK with `genai.Client(api_key=...)` (thread-safe, no global state) |
|
| 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 |
-
## Round 11 — 2026-03-06 Session 6 (8-Agent Final Pre-Release Audit + Cross-Check)
|
| 280 |
-
|
| 281 |
-
This round was the strictest final audit before public release: 8 parallel agents (core pipeline, ingestion/storage, SQL security, LLM/prompts, UIs/setup, readers/search, test suite, design docs/architecture) each read every line of their assigned files and cross-checked findings.
|
| 282 |
-
|
| 283 |
-
### HIGH Fixes
|
| 284 |
-
|
| 285 |
-
| # | Severity | File | Bug | Fix | Status |
|
| 286 |
-
|---|----------|------|-----|-----|--------|
|
| 287 |
-
| 84 | HIGH | `src/query_engine.py:96` | QU `max_tokens=256` too tight — SQL queries + reasoning fields truncate JSON output, causing silent fallback to vector-only search | Increased to `max_tokens=512` | FIXED |
|
| 288 |
-
| 85 | HIGH | `src/llm/gemini.py:25-26` | Gemini `generate()` returned error strings instead of raising exceptions, inconsistent with OpenAI/Anthropic — error strings leaked into correction loop as "responses" | Changed to `raise RuntimeError(...)` for API errors (ValueError for safety blocks still returns string) | FIXED |
|
| 289 |
-
| 86 | HIGH | `src/sql_retriever.py:184-214` | `make_fuzzy_query()` unsanitized value interpolation — captured regex values not escaped for single quotes, allowing SQL injection via crafted `WHERE` values | Added `.replace("'", "''")` escaping in both `_phrase_replace()` and `_word_replace()` | FIXED |
|
| 290 |
-
| 87 | HIGH | `src/config_loader.py:35-37` | Nested null YAML values crashed consumers — `vector_db:` with no value → `.get("vector_db", "chroma_db")` returns `None` → `TypeError` in `os.path.isabs(None)` | Changed top-level-only normalization to recursive `_normalize_nulls()` that handles all nested dicts | FIXED |
|
| 291 |
-
| 88 | HIGH | `setup.py:93-94` | `.env` parser used `strip('"')` which removes ALL leading/trailing quotes, not just one matched pair — could corrupt API keys starting/ending with quote chars | Changed to proper one-pair unwrap: check for matching pair then slice `[1:-1]` | FIXED |
|
| 292 |
-
| 89 | HIGH | `setup.py:100-102` | `.env` writer did not escape special chars in values — API keys containing `"` or `\` produced malformed `.env` files | Added `v.replace('\\', '\\\\').replace('"', '\\"')` before writing | FIXED |
|
| 293 |
-
| 90 | HIGH | `src/sql_retriever.py:41-42` | `sqlite_master` access not blocked — `SELECT * FROM sqlite_master` passed all validation, exposing full DDL schema | Added `sqlite_(master\|schema\|temp_master\|temp_schema)` check in `_validate_sql()` | FIXED |
|
| 294 |
-
| 91 | HIGH | `tests/test_verifier.py:65-66,95-96` | Wrong mock target — patched `src.kb_meta.load_kb_meta` but verifier imports `load_kb_meta_brief`; mocks were no-ops (only worked due to fallback chain) | Changed both patches to `src.kb_meta.load_kb_meta_brief` | FIXED |
|
| 295 |
-
| 92 | HIGH | `src/retriever.py:381` | SQL keyword fallback ran unconditionally for `route="sql"` even when vector search already found results — wasted computation | Added `and not db_results` condition: only run keyword SQL when vector fallback also found nothing | FIXED |
|
| 296 |
-
|
| 297 |
-
### MEDIUM Fixes
|
| 298 |
-
|
| 299 |
-
| # | Severity | File | Bug | Fix | Status |
|
| 300 |
-
|---|----------|------|-----|-----|--------|
|
| 301 |
-
| 93 | MEDIUM | `src/search/semantic_scholar.py:47,49` | `citationCount: null` from API → `None` in sort key → `TypeError: '<' not supported between NoneType and int` crash | Changed to `paper.get("citationCount") or 0` and `x.get("citation_count") or 0` | FIXED |
|
| 302 |
-
| 94 | MEDIUM | `app_web.py:123` | Stale KB welcome summary after re-ingest — `st.session_state.kb_welcome_summary` not cleared before `st.rerun()` | Added `st.session_state.pop("kb_welcome_summary", None)` before rerun | FIXED |
|
| 303 |
-
| 95 | MEDIUM | `app_web.py:256-257,267` | SQL result count missing from web UI status label — showed "0 local + 0 web" even when SQL returned rows | Added `n_sql` to both "Verified" and "Done" status labels, matching CLI behavior | FIXED |
|
| 304 |
-
| 96 | MEDIUM | `src/verifier.py:184` | References regex `\b(references\|sources)\b` matched casual mentions of "sources" in prose — citation audit checked entire response body instead of just References section | Changed to heading-specific regex: `^#+\s*(references\|sources)` or `^\*\*(references\|sources)\*\*` | FIXED |
|
| 305 |
-
| 97 | MEDIUM | `src/verifier.py:486` | Non-strict mode used stale `citation_warnings` from last loop iteration, not from final corrected response | Added `validate_citations(response, retrieval_result)` recomputation before building warning | FIXED |
|
| 306 |
-
| 98 | MEDIUM | `src/ingest.py:291-298` | Meta files deleted before overview regeneration — if `build_and_store_overview()` failed, KB lost self-awareness until next ingestion | Removed pre-deletion; `write_text()` in `build_and_store_overview()` already overwrites | FIXED |
|
| 307 |
-
| 99 | MEDIUM | `src/readers/docx.py:4-10` | DOCX reader only extracted paragraphs, silently dropping all embedded tables | Added table extraction loop: `doc.tables` → row cells joined with ` \| ` | FIXED |
|
| 308 |
-
|
| 309 |
-
### Known Issues Added
|
| 310 |
-
|
| 311 |
-
| # | Severity | File | Description | Reason |
|
| 312 |
-
|---|----------|------|-------------|--------|
|
| 313 |
-
| K18 | MEDIUM | `src/sql_retriever.py:17-24` | Comment stripping is naive (regex) — corrupts string literals containing `--` or `/* */` | Rare in LLM-generated queries; proper state-machine parser would be over-engineering |
|
| 314 |
-
| K19 | MEDIUM | `src/retriever.py:195-196, 267-269` | LIKE wildcard chars `%` and `_` not escaped in fallback/alt-column queries | Same as existing K7; low real-world impact |
|
| 315 |
-
| K20 | MEDIUM | `src/sql_retriever.py` | No SQL execution timeout — Cartesian products could hang | Same as existing K10; SQLite lacks built-in SELECT timeout |
|
| 316 |
-
| K21 | MEDIUM | `src/verifier.py:151-202` | `validate_citations` compares citation numbers against raw retrieval count, not References section entry count | Would require parsing References section to count entries; current check is conservative |
|
| 317 |
-
| K22 | LOW | `src/readers/csv_tab.py:11` | CSV encoding hardcoded to `utf-8-sig` — non-UTF-8 files get replacement chars | Would need `chardet` dependency or encoding detection |
|
| 318 |
-
| K23 | LOW | `src/sql_ingest.py:568-586` | Row-by-row SQL INSERT slow for large datasets | Would benefit from `executemany()` batching |
|
| 319 |
-
| K24 | LOW | `src/readers/excel.py:40, csv_tab.py:22` | `zip(headers, row)` silently truncates extra columns in ragged rows | Acceptable for most research datasets |
|
| 320 |
-
| K25 | LOW | `src/ingest.py:104` | `rglob("*")` follows symlinks — cyclic symlinks could cause infinite loop | Rare in knowledge_base directories |
|
| 321 |
-
| K26 | LOW | Test suite | Zero coverage: `validate_citations`, verification loop, `make_fuzzy_query`, `compute_similarity_flags`, `load_kb_meta_brief` | Test gap documented; non-blocking for release |
|
| 322 |
-
|
| 323 |
-
### Test Coverage Gaps (Confirmed)
|
| 324 |
-
|
| 325 |
-
| Priority | What | Description |
|
| 326 |
-
|----------|------|-------------|
|
| 327 |
-
| HIGH | `validate_citations()` | Layer 4.5 — zero direct tests |
|
| 328 |
-
| HIGH | Verification correction loop | Core feature — zero multi-iteration tests |
|
| 329 |
-
| HIGH | `make_fuzzy_query()` | SQL fuzzy fallback — zero tests |
|
| 330 |
-
| MEDIUM | `compute_similarity_flags()` | Layer 4 — zero tests |
|
| 331 |
-
| MEDIUM | `load_kb_meta_brief()` | Used by verifier — zero tests |
|
| 332 |
-
|
| 333 |
-
---
|
| 334 |
-
|
| 335 |
-
## Round 12 — 2026-03-06 Session 7 (Pre-Release Final Audit)
|
| 336 |
-
|
| 337 |
-
See `docs/AUDIT_BUGLOG.md` Round 12 section for full details.
|
| 338 |
-
33 bugs found across 3 tiers, all FIXED. Design score raised from 7.1/10 to 9/10.
|
| 339 |
-
5 NEW low-severity issues documented as KNOWN. 174/174 tests passing. **Released to GitHub.**
|
| 340 |
-
|
| 341 |
-
---
|
| 342 |
-
|
| 343 |
-
## Round 13 — 2026-03-07 (Post-Release Deep Audit)
|
| 344 |
-
|
| 345 |
-
**7 parallel audit agents** (design evaluation + core pipeline + verification stack + SQL layer + LLM/readers/search + application layer + test coverage). Full line-by-line review of all 25 source files, 17 test files, and all documentation.
|
| 346 |
-
|
| 347 |
-
**Design Score**: 8.5/10 → **9.1/10** (post-fix) | **Test Coverage Score**: 6/10
|
| 348 |
-
|
| 349 |
-
### Tier 1: Anti-Hallucination Integrity (4 bugs) — ALL FIXED
|
| 350 |
-
|
| 351 |
-
| # | Severity | File | Bug | Fix | Status |
|
| 352 |
-
|---|----------|------|-----|-----|--------|
|
| 353 |
-
| 100 | HIGH | `verifier.py` | **R13-T1-01**: Verification LLM persistent failure → `except Exception: continue` skips all iterations → non-strict returns unverified response. Full bypass of Layers 3–5. | Added `any_verification_ran` flag. If ALL verification LLM calls fail: strict mode refuses with system-failure message; non-strict adds prominent WARNING banner distinguishing "could not run" from "did not pass". | FIXED |
|
| 354 |
-
| 101 | HIGH | `verifier.py` | **R13-T1-02**: `validate_citations` SQL/web source matching uses wrong dict structure (dead code). Layer 4.5 only works for local sources. | SQL: extract source filename from context via regex. Web: changed to flat dict access `web_chunk.get("url", "")`. | FIXED |
|
| 355 |
-
| 102 | HIGH | `verifier.py` | **R13-T1-03**: Citation count uses unique source *files*, not citable *units*. False "fabricated reference" warning on multi-chunk single-file sources. | Changed `db_count = len(db_results)` (chunks) instead of `len(unique_db_sources)` (files). Each chunk is a citable unit. | FIXED |
|
| 356 |
-
| 103 | MEDIUM | `verifier.py` | **R13-T1-04**: `max_iterations: 0` with `enabled: true` silently disables verification. | Clamp `max_iterations = max(1, max_iterations)` with warning. Must use `enabled: false` to disable. Test updated. | FIXED |
|
| 357 |
-
|
| 358 |
-
### Tier 2: Pre-Release Quality (24 bugs) — ALL FIXED
|
| 359 |
-
|
| 360 |
-
| # | Severity | File | Bug | Fix | Status |
|
| 361 |
-
|---|----------|------|-----|-----|--------|
|
| 362 |
-
| 104 | HIGH | `retriever.py` | **R13-T2-01**: Missing LIKE wildcard ESCAPE in `_build_fallback_sql_query` and `_try_alternate_columns`. | Added `%`/`_` escaping and `ESCAPE '\'` clause. Initial fix had double-backslash bug; corrected by regression checker. | FIXED |
|
| 363 |
-
| 105 | HIGH | `gemini.py` | **R13-T2-02**: Gemini safety filter returns `"[Gemini blocked: ...]"` bypassing anti-hallucination stack. | Return `""` instead. Verifier's empty-response handler (lines 369-378) provides user-facing refusal. | FIXED |
|
| 364 |
-
| 106 | HIGH | `openai.py` | **R13-T2-03**: Reasoning model budget uncapped → API error with large `max_tokens`. | `reasoning_budget = min(max(max_tokens * 4, 4096), 128000)`. | FIXED |
|
| 365 |
-
| 107 | HIGH | `llm/__init__.py` | **R13-T2-04**: Non-numeric temperature/max_tokens crashes with opaque ValueError. | Wrapped in try/except with defaults (0.0 and 2048). | FIXED |
|
| 366 |
-
| 108 | HIGH | `app_cli.py` | **R13-T2-05**: After clarification, raw `user_input` re-appended → duplicate history entries. | Use `display_query` (post-QU, includes clarification context) instead of raw `user_input`. | FIXED |
|
| 367 |
-
| 109 | MEDIUM | `llm/__init__.py` | **R13-T2-06**: Anthropic temp > 1.0 → API error. | Added provider-specific clamp: `if provider == "anthropic": temperature = min(temperature, 1.0)`. | FIXED |
|
| 368 |
-
| 110 | MEDIUM | `sql_retriever.py` | **R13-T2-07**: No SQL query timeout. `CROSS JOIN` hangs indefinitely. | Added `set_progress_handler` with 10-second abort. `import time` at module level. | FIXED |
|
| 369 |
-
| 111 | MEDIUM | `app_cli.py` | **R13-T2-08**: Rich markup injection via brackets in error messages/queries. | Added `rich_escape()` to all 5 interpolated f-strings in the RAG pipeline loop. | FIXED |
|
| 370 |
-
| 112 | MEDIUM | `readers/*.py` | **R13-T2-09**: No try/except in readers. Corrupt file aborts ingestion. | Wrapped `read_pdf`, `read_docx`, `read_excel`, `read_stata`, `read_rdata`, `read_spss` in try/except. | FIXED |
|
| 371 |
-
| 113 | MEDIUM | `retriever.py`, `app_cli.py`, `app_web.py` | **R13-T2-10**: `_collection_cache` never invalidated after re-ingestion. | Added `clear_collection_cache()` function; called after ingestion in both CLI and web. | FIXED |
|
| 372 |
-
| 114 | MEDIUM | `app_web.py` | **R13-T2-11**: Web UI messages grow without limit. | Capped at 200 entries (100 Q&A pairs) after each append cycle. | FIXED |
|
| 373 |
-
| 115 | MEDIUM | `setup.py` | **R13-T2-12**: Setup wizard overwrites config without confirmation. | Added existence check + `[y/N]` prompt before overwriting `config.yaml`. | FIXED |
|
| 374 |
-
| 116 | MEDIUM | `prompts.py` | **R13-T2-13**: Prompt injection via KB delimiter mimicry. | Sanitize context: replace 69-char `=` sequences with Unicode `≡` before injection. | FIXED |
|
| 375 |
-
| 117 | MEDIUM | `verifier.py` | **R13-T2-14**: References regex matches `## Data Sources` etc. | Anchored regex with `\s*$` to require standalone headers only. | FIXED |
|
| 376 |
-
| 118 | MEDIUM | `verifier.py` | **R13-T2-15**: Correction prompt includes full previous response. | Truncate to 1500 chars. Instruction: "Rewrite COMPLETE response from scratch". | FIXED |
|
| 377 |
-
| 119 | MEDIUM | `verifier.py` | **R13-T2-16**: Initial `generate()` exception propagates. | Wrapped in try/except; returns `refused: True` with error message. | FIXED |
|
| 378 |
-
| 120 | MEDIUM | `openai.py` | **R13-T2-17**: Reasoning model detection via brittle prefix match. | Changed to `model in ("o1","o3","o4") or model.startswith(("o1-","o3-","o4-"))`. | FIXED |
|
| 379 |
-
| 121 | MEDIUM | `sql_retriever.py` | **R13-T2-18**: `make_fuzzy_query` regex matches non-column contexts. | Pattern now requires quoted identifiers or valid SQL column names. | FIXED |
|
| 380 |
-
| 122 | MEDIUM | `sql_retriever.py` | **R13-T2-19**: `REPLACE()` function falsely blocked. | Removed `REPLACE` from `_DANGEROUS_KEYWORDS`. Statement form blocked by SELECT-only check. | FIXED |
|
| 381 |
-
| 123 | MEDIUM | `config_loader.py` | **R13-T2-20**: Non-dict YAML → `AttributeError`. | Added `isinstance(cfg, dict)` check with clear ValueError message. | FIXED |
|
| 382 |
-
| 124 | MEDIUM | `app_web.py` | **R13-T2-21**: Model list cache stale after provider change. | Added provider-change detection that clears all model caches. | FIXED |
|
| 383 |
-
| 125 | MEDIUM | `app_web.py` | **R13-T2-22**: Web UI uses slow LLM call for KB welcome. | Changed to `load_kb_meta_brief()` (file read) matching CLI behavior. | FIXED |
|
| 384 |
-
| 126 | MEDIUM | `app_web.py` | **R13-T2-23**: Re-ingestion blocks without progress feedback. | Added `st.info()` message before spinner to set expectations. | FIXED |
|
| 385 |
-
| 127 | MEDIUM | `setup.py` | **R13-T2-24**: Empty API key warning not actionable. | Improved message: "Set it later by editing .env or re-running: python setup.py". | FIXED |
|
| 386 |
-
|
| 387 |
-
### Tier 3: Post-Release Improvements (20 bugs)
|
| 388 |
-
|
| 389 |
-
| # | Severity | File | Bug | Status |
|
| 390 |
-
|---|----------|------|-----|--------|
|
| 391 |
-
| 128 | MED | `openai.py:9`, `anthropic.py:9`, `gemini.py:16` | **R13-T3-01**: New API client per `generate()` call. No connection reuse. | OPEN |
|
| 392 |
-
| 129 | MED | `llm/openai.py:52`, `anthropic.py:32`, `gemini.py:48` | **R13-T3-02**: `list_models()` returns hardcoded list, not live API fetch. Design doc says otherwise. | OPEN |
|
| 393 |
-
| 130 | MED | `retriever.py:282-285` | **R13-T3-03**: `_try_alternate_columns` quote escaping logic fragile and non-obvious. | OPEN |
|
| 394 |
-
| 131 | MED | `sql_retriever.py:54` | **R13-T3-04**: `sqlite_master` check on `stripped` instead of `unquoted`. False positives. | OPEN |
|
| 395 |
-
| 132 | MED | `readers/stata.py:28`, `rdata.py:15` | **R13-T3-05**: `df.iterrows()` 10-100x slower than `itertuples()`. Large datasets take minutes. | OPEN |
|
| 396 |
-
| 133 | LOW | `sql_retriever.py:26-33` | **R13-T3-06**: `_strip_sql_comments` removes `--` inside string literals. Confirmed from R12 NEW-04. | OPEN |
|
| 397 |
-
| 134 | LOW | `verifier.py:429` | **R13-T3-07**: Verification LLM system prompt minimal. No anti-hallucination rules for verifier itself. | OPEN |
|
| 398 |
-
| 135 | LOW | `verifier.py:25-34` | **R13-T3-08**: Warning phrases easily evaded by paraphrasing. Only 8 exact-match phrases. | OPEN |
|
| 399 |
-
| 136 | LOW | `verifier.py:126` | **R13-T3-09**: Term overlap excludes 2-char acronyms (AI, UK, EU, US, UN). | OPEN |
|
| 400 |
-
| 137 | LOW | `verifier.py:181` | **R13-T3-10**: Citation `[0]` never detected as invalid. | OPEN |
|
| 401 |
-
| 138 | LOW | `readers/csv_tab.py:11`, `text.py:5` | **R13-T3-11**: Hardcoded UTF-8. Non-UTF-8 files get replacement chars. | OPEN |
|
| 402 |
-
| 139 | LOW | `readers/docx.py:24` | **R13-T3-12**: All DOCX content mapped to `page: 1`. | OPEN |
|
| 403 |
-
| 140 | LOW | `readers/pdf.py:9` | **R13-T3-13**: Scanned PDFs silently return empty list. No warning. | OPEN |
|
| 404 |
-
| 141 | LOW | `readers/excel.py:15` | **R13-T3-14**: Unevaluated formulas appear as `None`, silently dropped. | OPEN |
|
| 405 |
-
| 142 | LOW | `search/semantic_scholar.py:15-28` | **R13-T3-15**: Retries on non-retriable 4xx errors. Wastes 17s. | OPEN |
|
| 406 |
-
| 143 | LOW | `search/__init__.py:10-12` | **R13-T3-16**: Unknown backend names silently return `[]`. | OPEN |
|
| 407 |
-
| 144 | LOW | `setup.py:96-100` | **R13-T3-17**: `.env` unescape wrong order. Theoretical corruption. | OPEN |
|
| 408 |
-
| 145 | LOW | `config_loader.py:20-22` | **R13-T3-18**: `.env` loaded before config existence check. | OPEN |
|
| 409 |
-
| 146 | LOW | `app_web.py:288` | **R13-T3-19**: Browser tab title hardcoded "ResearchBot". | OPEN |
|
| 410 |
-
| 147 | LOW | `verifier.py:460-462` | **R13-T3-20**: Correction call uses same temperature as initial. Should force 0.0. | OPEN |
|
| 411 |
-
|
| 412 |
-
### Test Coverage Gaps (Round 13 — confirmed + expanded)
|
| 413 |
-
|
| 414 |
-
| Priority | What | File | Status |
|
| 415 |
-
|----------|------|------|--------|
|
| 416 |
-
| CRITICAL | `validate_citations()` — Layer 4.5 | `verifier.py:152-230` | OPEN |
|
| 417 |
-
| CRITICAL | Verification correction loop | `verifier.py:407-520` | OPEN |
|
| 418 |
-
| CRITICAL | `make_fuzzy_query()` | `sql_retriever.py:181-237` | OPEN |
|
| 419 |
-
| HIGH | `compute_similarity_flags()` — Layer 4 | `verifier.py:101-149` | OPEN |
|
| 420 |
-
| HIGH | `_strip_quoted()` / `_strip_sql_comments()` | `sql_retriever.py:17-33` | OPEN |
|
| 421 |
-
| HIGH | Empty LLM response in `verify_and_respond` | `verifier.py:354-364` | OPEN |
|
| 422 |
-
| HIGH | `build_and_store_overview()` orchestrator | `kb_meta.py:464-492` | OPEN |
|
| 423 |
-
| MEDIUM | `generate_brief_overview()` | `kb_meta.py:385-422` | OPEN |
|
| 424 |
-
| MEDIUM | `load_kb_meta_brief()` fallback | `kb_meta.py:434-445` | OPEN |
|
| 425 |
-
| MEDIUM | `summarize_kb_for_welcome()` | `kb_meta.py:514-552` | OPEN |
|
| 426 |
-
| MEDIUM | `_build_fallback_sql_query` with real data | `retriever.py:159-217` | OPEN |
|
| 427 |
-
| MEDIUM | `_try_alternate_columns` with real data | `retriever.py:220-292` | OPEN |
|
| 428 |
-
| MEDIUM | `_validate_sql` edge cases | `sql_retriever.py:36-59` | OPEN |
|
| 429 |
-
| MEDIUM | `format_sql_results_as_context` truncation | `sql_retriever.py:116-167` | OPEN |
|
| 430 |
-
| MEDIUM | `_normalize_nulls` | `config_loader.py:36-42` | OPEN |
|
| 431 |
-
|
| 432 |
-
### Cross-Agent Consensus (confirmed by 2+ agents independently)
|
| 433 |
-
|
| 434 |
-
| Bug | Agents | Confidence |
|
| 435 |
-
|-----|--------|-----------|
|
| 436 |
-
| `validate_citations` dead code for SQL + web (#101) | Core Pipeline, Application, Verification | **Very High** |
|
| 437 |
-
| Missing LIKE ESCAPE in SQL fallbacks (#104) | SQL, Core Pipeline, Design | **Very High** |
|
| 438 |
-
| Stale `_collection_cache` after re-ingestion (#113) | Core Pipeline, Design | **High** |
|
| 439 |
-
| Verification LLM failure → silent bypass (#100) | Verification, Core Pipeline | **High** |
|
| 440 |
-
| Missing reader exception handling (#112) | LLM/Readers, Design | **High** |
|
| 441 |
-
|
| 442 |
-
### Recommended Fix Order
|
| 443 |
-
|
| 444 |
-
**Phase 1 — Anti-hallucination integrity (#100–#103):**
|
| 445 |
-
Fix verification bypass, validate_citations data structure, citation counting, max_iterations guard.
|
| 446 |
-
|
| 447 |
-
**Phase 2 — High-severity T2 bugs (#104–#108):**
|
| 448 |
-
LIKE ESCAPE, Gemini blocked response, reasoning model overflow, config validation, history corruption.
|
| 449 |
-
|
| 450 |
-
**Phase 3 — Test coverage (3 CRITICAL gaps):**
|
| 451 |
-
Tests for validate_citations, verification correction loop, make_fuzzy_query.
|
| 452 |
-
|
| 453 |
-
**Phase 4 — Remaining T2 MEDIUM bugs (#109–#127):**
|
| 454 |
-
Address in order of impact.
|
| 455 |
-
|
| 456 |
-
**Phase 5 — T3 improvements (#128–#147):**
|
| 457 |
-
Address as time permits.
|
| 458 |
-
|
| 459 |
-
---
|
| 460 |
-
|
| 461 |
-
## Round 14 — 2026-03-07 (Tier 1 + Tier 2 Bug Fix Sprint)
|
| 462 |
-
|
| 463 |
-
**6 parallel agents**: 4 fix agents (T1, T2-HIGH, T2-MED-A, T2-MED-B) + 1 regression checker + 1 design guardian.
|
| 464 |
-
|
| 465 |
-
- Fixed all 28 Tier 1 + Tier 2 bugs (#100–#127)
|
| 466 |
-
- Regression checker found 1 CRITICAL issue in Bug #104 fix (double-backslash in ESCAPE clause) — corrected
|
| 467 |
-
- Also fixed `spss.py` missing try/except (pre-existing gap found by regression checker)
|
| 468 |
-
- Design Guardian assessment: design score 8.5 → 9.1/10, recommendation: SHIP
|
| 469 |
-
- Updated test for Bug #103 (renamed to `test_max_iterations_zero_clamps_to_one`)
|
| 470 |
-
- 174/174 tests passing
|
| 471 |
-
|
| 472 |
-
---
|
| 473 |
-
|
| 474 |
-
## Statistics
|
| 475 |
-
|
| 476 |
-
| Metric | Count |
|
| 477 |
-
|--------|-------|
|
| 478 |
-
| Total bugs found (all rounds) | ~216 (48 new in Round 13) |
|
| 479 |
-
| Total bugs fixed (all rounds) | ~196 (168 in R1–R12, 28 in R14) |
|
| 480 |
-
| Round 13 T1+T2 bugs | 28 FIXED (was 28 OPEN) |
|
| 481 |
-
| Round 13 T3 bugs remaining | 20 OPEN (5 MEDIUM, 15 LOW) |
|
| 482 |
-
| Audit rounds completed | 14 (50+ total agent dispatches) |
|
| 483 |
-
| Test count | 174 passing |
|
| 484 |
-
| Test files | 17 |
|
| 485 |
-
| Source files modified in R14 | 16 (verifier, retriever, gemini, openai, llm/__init__, app_cli, app_web, sql_retriever, config_loader, prompts, setup, pdf, docx, excel, stata, rdata, spss) |
|
| 486 |
-
| Design score | 9.1/10 (up from 8.5) |
|
| 487 |
-
| CRITICAL test gaps | 3 (validate_citations, correction loop, make_fuzzy_query) |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
tests/test_retriever.py
DELETED
|
@@ -1,247 +0,0 @@
|
|
| 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_builds_fresh_query(
|
| 207 |
-
mock_build_fallback, mock_run_sql, mock_vectordb, mock_search,
|
| 208 |
-
):
|
| 209 |
-
"""When route='both' and LLM sql_query failed, build a fresh keyword query."""
|
| 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 |
-
# T2-03 fix: _build_fallback_sql_query IS called to build a fresh keyword
|
| 219 |
-
# query instead of re-using the already-failed LLM sql_query
|
| 220 |
-
mock_build_fallback.assert_called_once_with("some data", _BASE_CFG)
|
| 221 |
-
# _run_sql_retrieval called twice: once for LLM query, once for fresh fallback
|
| 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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|