Spaces:
Paused
Paused
Commit ·
f9f7706
1
Parent(s): 8519048
fix: Round 17 audit — critical/high bug fixes (5 bugs, cross-checked)
Browse files- H1: validate_citations regex now matches References:/Reference/Sources (Primary)
- H2: _strip_sql_comments rewritten as state machine, preserves --//**/ inside strings
- H3: SQL citation count fixed: 1 source per query, not 1 per row
- M1: Context budget now accounts for web context size before local allocation
- M2: max_context_chars default aligned to 12000 (was 0 in code vs 12000 in docs)
10 agents dispatched: 5 audit + 3 fix + 2 cross-check. 175/175 tests passing.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- .gitignore +16 -0
- setup.py +2 -1
- src/retriever.py +39 -8
- src/sql_retriever.py +41 -7
- src/verifier.py +8 -3
.gitignore
ADDED
|
@@ -0,0 +1,16 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
.env
|
| 2 |
+
config.yaml
|
| 3 |
+
chroma_db/
|
| 4 |
+
sql_db/
|
| 5 |
+
__pycache__/
|
| 6 |
+
*.pyc
|
| 7 |
+
.DS_Store
|
| 8 |
+
*.egg-info/
|
| 9 |
+
dist/
|
| 10 |
+
build/
|
| 11 |
+
.pytest_cache/
|
| 12 |
+
knowledge_base/*
|
| 13 |
+
!knowledge_base/.gitkeep
|
| 14 |
+
CLAUDE.md
|
| 15 |
+
docs/
|
| 16 |
+
tests/
|
setup.py
CHANGED
|
@@ -48,8 +48,9 @@ def generate_config(
|
|
| 48 |
"retrieval": {
|
| 49 |
"chunk_size": 1000,
|
| 50 |
"chunk_overlap": 100,
|
| 51 |
-
"top_k":
|
| 52 |
"max_distance": 0.55,
|
|
|
|
| 53 |
},
|
| 54 |
"web_search": {
|
| 55 |
"enabled": web_search,
|
|
|
|
| 48 |
"retrieval": {
|
| 49 |
"chunk_size": 1000,
|
| 50 |
"chunk_overlap": 100,
|
| 51 |
+
"top_k": 20,
|
| 52 |
"max_distance": 0.55,
|
| 53 |
+
"max_context_chars": 12000,
|
| 54 |
},
|
| 55 |
"web_search": {
|
| 56 |
"enabled": web_search,
|
src/retriever.py
CHANGED
|
@@ -43,7 +43,7 @@ def retrieve_from_vectordb(query: str, cfg: dict) -> list[dict]:
|
|
| 43 |
base can still be answered.
|
| 44 |
"""
|
| 45 |
retrieval_cfg = cfg.get("retrieval", {})
|
| 46 |
-
top_k = retrieval_cfg.get("top_k",
|
| 47 |
max_distance = retrieval_cfg.get("max_distance", 0.55)
|
| 48 |
collection = _get_cached_collection(cfg)
|
| 49 |
|
|
@@ -93,32 +93,59 @@ def retrieve_from_vectordb(query: str, cfg: dict) -> list[dict]:
|
|
| 93 |
return chunks
|
| 94 |
|
| 95 |
|
| 96 |
-
def format_db_results_as_context(chunks: list[dict]) -> str:
|
| 97 |
-
"""Format vector DB results with CHUNK-LOCAL IDs for citation anchoring.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 98 |
if not chunks:
|
| 99 |
return ""
|
| 100 |
|
| 101 |
-
|
|
|
|
|
|
|
| 102 |
for i, chunk in enumerate(chunks, 1):
|
| 103 |
meta = chunk.get("metadata", {})
|
| 104 |
dataset = meta.get("dataset", "")
|
| 105 |
dataset_label = f" [Dataset: {dataset}]" if dataset else ""
|
| 106 |
source_path = meta.get('source', 'unknown')
|
| 107 |
-
|
| 108 |
f"[CHUNK-LOCAL-{i:03d}] From: {meta.get('source', 'unknown')}, "
|
| 109 |
f"Page/Section {meta.get('page', '?')}{dataset_label}\n"
|
| 110 |
f" Path: {source_path}\n"
|
| 111 |
f" {chunk.get('text', '')}\n"
|
| 112 |
)
|
|
|
|
|
|
|
|
|
|
|
|
|
| 113 |
return "\n".join(parts)
|
| 114 |
|
| 115 |
|
| 116 |
def build_combined_context(
|
| 117 |
db_results: list[dict], web_results: list[dict], sql_context: str = "",
|
|
|
|
| 118 |
) -> str:
|
| 119 |
-
"""Build combined context string from local, SQL, and web results.
|
| 120 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 121 |
web_context = format_web_results_as_context(web_results)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 122 |
|
| 123 |
has_local = bool(db_results) or bool(sql_context)
|
| 124 |
has_web = bool(web_results)
|
|
@@ -428,7 +455,11 @@ def retrieve(
|
|
| 428 |
max_results *= 2
|
| 429 |
web_results = search(query, backend=backend, limit=max_results)
|
| 430 |
|
| 431 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
| 432 |
has_sources = bool(db_results) or bool(web_results) or bool(sql_rows)
|
| 433 |
|
| 434 |
return {
|
|
|
|
| 43 |
base can still be answered.
|
| 44 |
"""
|
| 45 |
retrieval_cfg = cfg.get("retrieval", {})
|
| 46 |
+
top_k = retrieval_cfg.get("top_k", 20)
|
| 47 |
max_distance = retrieval_cfg.get("max_distance", 0.55)
|
| 48 |
collection = _get_cached_collection(cfg)
|
| 49 |
|
|
|
|
| 93 |
return chunks
|
| 94 |
|
| 95 |
|
| 96 |
+
def format_db_results_as_context(chunks: list[dict], max_chars: int = 0) -> str:
|
| 97 |
+
"""Format vector DB results with CHUNK-LOCAL IDs for citation anchoring.
|
| 98 |
+
|
| 99 |
+
Args:
|
| 100 |
+
chunks: Retrieved chunks sorted by distance (most relevant first).
|
| 101 |
+
max_chars: If > 0, stop adding chunks once total context exceeds
|
| 102 |
+
this character budget. Chunks already sorted by relevance
|
| 103 |
+
ensure the most important content is kept.
|
| 104 |
+
"""
|
| 105 |
if not chunks:
|
| 106 |
return ""
|
| 107 |
|
| 108 |
+
header = "=== Local Document Results (PRIMARY — always trust these over web sources) ===\n"
|
| 109 |
+
parts = [header]
|
| 110 |
+
total_chars = len(header)
|
| 111 |
for i, chunk in enumerate(chunks, 1):
|
| 112 |
meta = chunk.get("metadata", {})
|
| 113 |
dataset = meta.get("dataset", "")
|
| 114 |
dataset_label = f" [Dataset: {dataset}]" if dataset else ""
|
| 115 |
source_path = meta.get('source', 'unknown')
|
| 116 |
+
entry = (
|
| 117 |
f"[CHUNK-LOCAL-{i:03d}] From: {meta.get('source', 'unknown')}, "
|
| 118 |
f"Page/Section {meta.get('page', '?')}{dataset_label}\n"
|
| 119 |
f" Path: {source_path}\n"
|
| 120 |
f" {chunk.get('text', '')}\n"
|
| 121 |
)
|
| 122 |
+
if max_chars > 0 and total_chars + len(entry) > max_chars and i > 1:
|
| 123 |
+
break
|
| 124 |
+
parts.append(entry)
|
| 125 |
+
total_chars += len(entry)
|
| 126 |
return "\n".join(parts)
|
| 127 |
|
| 128 |
|
| 129 |
def build_combined_context(
|
| 130 |
db_results: list[dict], web_results: list[dict], sql_context: str = "",
|
| 131 |
+
max_context_chars: int = 0,
|
| 132 |
) -> str:
|
| 133 |
+
"""Build combined context string from local, SQL, and web results.
|
| 134 |
+
|
| 135 |
+
When *max_context_chars* > 0, the local-chunk context is truncated to
|
| 136 |
+
fit within the budget (after accounting for SQL and web context).
|
| 137 |
+
SQL and web context are never truncated; the budget applies to local chunks.
|
| 138 |
+
"""
|
| 139 |
+
# SQL context is precise and compact — always include in full.
|
| 140 |
+
# Web context is supplementary — include in full but account for its size.
|
| 141 |
+
# Allocate remaining budget to local chunks.
|
| 142 |
web_context = format_web_results_as_context(web_results)
|
| 143 |
+
local_budget = 0
|
| 144 |
+
if max_context_chars > 0:
|
| 145 |
+
reserved = len(sql_context) + len(web_context)
|
| 146 |
+
local_budget = max(max_context_chars - reserved, 2000)
|
| 147 |
+
|
| 148 |
+
db_context = format_db_results_as_context(db_results, max_chars=local_budget)
|
| 149 |
|
| 150 |
has_local = bool(db_results) or bool(sql_context)
|
| 151 |
has_web = bool(web_results)
|
|
|
|
| 455 |
max_results *= 2
|
| 456 |
web_results = search(query, backend=backend, limit=max_results)
|
| 457 |
|
| 458 |
+
max_context_chars = cfg.get("retrieval", {}).get("max_context_chars", 12000)
|
| 459 |
+
combined_context = build_combined_context(
|
| 460 |
+
db_results, web_results, sql_context=sql_context,
|
| 461 |
+
max_context_chars=max_context_chars,
|
| 462 |
+
)
|
| 463 |
has_sources = bool(db_results) or bool(web_results) or bool(sql_rows)
|
| 464 |
|
| 465 |
return {
|
src/sql_retriever.py
CHANGED
|
@@ -25,13 +25,47 @@ def _strip_quoted(sql: str) -> str:
|
|
| 25 |
|
| 26 |
|
| 27 |
def _strip_sql_comments(sql: str) -> str:
|
| 28 |
-
"""Remove SQL comments (single-line -- and multi-line /* */) from a query.
|
| 29 |
-
|
| 30 |
-
|
| 31 |
-
|
| 32 |
-
|
| 33 |
-
|
| 34 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 35 |
|
| 36 |
|
| 37 |
def _validate_sql(sql: str) -> bool:
|
|
|
|
| 25 |
|
| 26 |
|
| 27 |
def _strip_sql_comments(sql: str) -> str:
|
| 28 |
+
"""Remove SQL comments (single-line -- and multi-line /* */) from a query.
|
| 29 |
+
|
| 30 |
+
Respects single-quoted string literals: ``--`` and ``/* */`` inside
|
| 31 |
+
quotes are preserved.
|
| 32 |
+
"""
|
| 33 |
+
result = []
|
| 34 |
+
i = 0
|
| 35 |
+
n = len(sql)
|
| 36 |
+
while i < n:
|
| 37 |
+
# Single-quoted string literal — copy verbatim (handles '' escaping)
|
| 38 |
+
if sql[i] == "'":
|
| 39 |
+
result.append("'")
|
| 40 |
+
i += 1
|
| 41 |
+
while i < n:
|
| 42 |
+
if sql[i] == "'" and i + 1 < n and sql[i + 1] == "'":
|
| 43 |
+
result.append("''")
|
| 44 |
+
i += 2
|
| 45 |
+
elif sql[i] == "'":
|
| 46 |
+
result.append("'")
|
| 47 |
+
i += 1
|
| 48 |
+
break
|
| 49 |
+
else:
|
| 50 |
+
result.append(sql[i])
|
| 51 |
+
i += 1
|
| 52 |
+
# Block comment
|
| 53 |
+
elif sql[i] == '/' and i + 1 < n and sql[i + 1] == '*':
|
| 54 |
+
i += 2
|
| 55 |
+
while i < n:
|
| 56 |
+
if sql[i] == '*' and i + 1 < n and sql[i + 1] == '/':
|
| 57 |
+
i += 2
|
| 58 |
+
break
|
| 59 |
+
i += 1
|
| 60 |
+
# Line comment
|
| 61 |
+
elif sql[i] == '-' and i + 1 < n and sql[i + 1] == '-':
|
| 62 |
+
i += 2
|
| 63 |
+
while i < n and sql[i] != '\n':
|
| 64 |
+
i += 1
|
| 65 |
+
else:
|
| 66 |
+
result.append(sql[i])
|
| 67 |
+
i += 1
|
| 68 |
+
return ''.join(result)
|
| 69 |
|
| 70 |
|
| 71 |
def _validate_sql(sql: str) -> bool:
|
src/verifier.py
CHANGED
|
@@ -168,14 +168,19 @@ def validate_citations(response: str, retrieval_result: dict) -> list[str]:
|
|
| 168 |
db_results = retrieval_result.get("db_results", [])
|
| 169 |
db_count = len(db_results)
|
| 170 |
web_count = len(retrieval_result.get("web_results", []))
|
| 171 |
-
|
|
|
|
|
|
|
| 172 |
total_sources = db_count + web_count + sql_count
|
| 173 |
|
| 174 |
if total_sources == 0:
|
| 175 |
return warnings
|
| 176 |
|
| 177 |
# Extract citation numbers from response BODY only (not References section)
|
| 178 |
-
refs_split = re.split(
|
|
|
|
|
|
|
|
|
|
| 179 |
body_text = refs_split[0] if refs_split else response
|
| 180 |
citation_nums = set(int(m) for m in re.findall(r"\[(\d+)\]", body_text))
|
| 181 |
if not citation_nums:
|
|
@@ -190,7 +195,7 @@ def validate_citations(response: str, retrieval_result: dict) -> list[str]:
|
|
| 190 |
|
| 191 |
# Check that source file names from retrieval appear in the References section
|
| 192 |
refs_match = re.search(
|
| 193 |
-
r"(?im)(?:^#+\s*(?:references|sources)\s*$|^\*\*(?:references|sources)\*\*\s*$).*",
|
| 194 |
response, re.DOTALL,
|
| 195 |
)
|
| 196 |
if refs_match:
|
|
|
|
| 168 |
db_results = retrieval_result.get("db_results", [])
|
| 169 |
db_count = len(db_results)
|
| 170 |
web_count = len(retrieval_result.get("web_results", []))
|
| 171 |
+
# SQL rows come from a single table query — count as 1 citable source,
|
| 172 |
+
# not one per row, since the LLM cites the table as a single reference.
|
| 173 |
+
sql_count = 1 if retrieval_result.get("sql_results") else 0
|
| 174 |
total_sources = db_count + web_count + sql_count
|
| 175 |
|
| 176 |
if total_sources == 0:
|
| 177 |
return warnings
|
| 178 |
|
| 179 |
# Extract citation numbers from response BODY only (not References section)
|
| 180 |
+
refs_split = re.split(
|
| 181 |
+
r'(?im)^#+\s*(?:references?|sources?)[\s:]*(?:\(.*?\))?\s*$|^\*\*(?:references?|sources?)[\s:]*(?:\(.*?\))?\*\*\s*$',
|
| 182 |
+
response
|
| 183 |
+
)
|
| 184 |
body_text = refs_split[0] if refs_split else response
|
| 185 |
citation_nums = set(int(m) for m in re.findall(r"\[(\d+)\]", body_text))
|
| 186 |
if not citation_nums:
|
|
|
|
| 195 |
|
| 196 |
# Check that source file names from retrieval appear in the References section
|
| 197 |
refs_match = re.search(
|
| 198 |
+
r"(?im)(?:^#+\s*(?:references?|sources?)[\s:]*(?:\(.*?\))?\s*$|^\*\*(?:references?|sources?)[\s:]*(?:\(.*?\))?\*\*\s*$).*",
|
| 199 |
response, re.DOTALL,
|
| 200 |
)
|
| 201 |
if refs_match:
|