Omar Solano Claude Fable 5 commited on
Commit
e0d6ca8
·
unverified ·
1 Parent(s): 0c86698

fix(retrieval): store the BM25 index as gzipped JSON instead of pickle (#13)

Browse files

HF's malware scanner flagged the public bundle's pkl as unsafe: BM25
vocabulary tokens from Python docs ("subprocess", "urllib.request", ...)
are byte-identical to the STACK_GLOBAL import operands its signatures
hunt for. A pickle also asks public downloaders to trust arbitrary code
execution on load.

The index is now a versioned gzipped-JSON payload (68 MB -> 21 MB) with
full parity verified against the old pkl (equal postings, frequencies,
lengths; identical search scores). Both HF bundles already ship the
.json.gz and the .pkl was removed; stale local bundles self-heal because
_bundle_complete() checks the new path.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

app/chroma_rag.py CHANGED
@@ -1,5 +1,6 @@
1
  from __future__ import annotations
2
 
 
3
  import json
4
  import logging
5
  import math
@@ -1068,18 +1069,50 @@ def tokenize_for_bm25(text: str) -> list[str]:
1068
  return tokens
1069
 
1070
 
 
 
 
 
 
 
 
1071
  def save_bm25_index(index: BM25Index, output_file: str) -> None:
1072
  ensure_parent_dir(output_file)
1073
- with open(output_file, "wb") as handle:
1074
- pickle.dump(index, handle)
 
 
 
 
 
 
 
 
 
 
1075
 
1076
 
1077
  def load_bm25_index(path: str) -> BM25Index | None:
1078
  if not os.path.exists(path):
1079
  return None
1080
  try:
1081
- with open(path, "rb") as handle:
1082
- index = pickle.load(handle)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1083
  except Exception as exc:
1084
  logger.warning(
1085
  "Failed to load BM25 index; falling back to dense-only retrieval. "
@@ -1089,14 +1122,7 @@ def load_bm25_index(path: str) -> BM25Index | None:
1089
  exc,
1090
  )
1091
  return None
1092
- if isinstance(index, BM25Index):
1093
- return index
1094
- logger.warning(
1095
- "BM25 index has unexpected type %s; falling back to dense-only retrieval. path=%s",
1096
- type(index).__name__,
1097
- path,
1098
- )
1099
- return None
1100
 
1101
 
1102
  def default_bm25_index_path(document_dict_path: str) -> str:
@@ -1104,8 +1130,8 @@ def default_bm25_index_path(document_dict_path: str) -> str:
1104
  name = path.name
1105
  if name.startswith("document_dict_") and name.endswith(".pkl"):
1106
  source = name.removeprefix("document_dict_").removesuffix(".pkl")
1107
- return str(path.with_name(f"bm25_index_{source}.pkl"))
1108
- return str(path.with_name("bm25_index.pkl"))
1109
 
1110
 
1111
  def result_dedupe_key(result: SearchResult) -> str:
 
1
  from __future__ import annotations
2
 
3
+ import gzip
4
  import json
5
  import logging
6
  import math
 
1069
  return tokens
1070
 
1071
 
1072
+ # Gzipped JSON, not pickle: the index ships in public HF bundles, and a pickle
1073
+ # full of Python-doc vocabulary tokens ("subprocess", "urllib.request", ...)
1074
+ # both trips Hub malware scanners and asks downloaders to trust arbitrary code
1075
+ # execution on load. Bump the version on any incompatible payload change.
1076
+ BM25_INDEX_FORMAT_VERSION = 1
1077
+
1078
+
1079
  def save_bm25_index(index: BM25Index, output_file: str) -> None:
1080
  ensure_parent_dir(output_file)
1081
+ payload = {
1082
+ "format_version": BM25_INDEX_FORMAT_VERSION,
1083
+ "k1": index.k1,
1084
+ "b": index.b,
1085
+ "average_document_length": index.average_document_length,
1086
+ "document_lengths": index.document_lengths,
1087
+ "document_frequencies": index.document_frequencies,
1088
+ "postings": index.postings,
1089
+ "records": [asdict(record) for record in index.records],
1090
+ }
1091
+ with gzip.open(output_file, "wt", encoding="utf-8") as handle:
1092
+ json.dump(payload, handle, ensure_ascii=False, separators=(",", ":"))
1093
 
1094
 
1095
  def load_bm25_index(path: str) -> BM25Index | None:
1096
  if not os.path.exists(path):
1097
  return None
1098
  try:
1099
+ with gzip.open(path, "rt", encoding="utf-8") as handle:
1100
+ payload = json.load(handle)
1101
+ version = payload.get("format_version")
1102
+ if version != BM25_INDEX_FORMAT_VERSION:
1103
+ raise ValueError(f"unsupported format_version={version!r}")
1104
+ index = BM25Index(
1105
+ records=[ChunkRecord(**record) for record in payload["records"]],
1106
+ postings={
1107
+ term: [(entry[0], entry[1]) for entry in entries]
1108
+ for term, entries in payload["postings"].items()
1109
+ },
1110
+ document_frequencies=payload["document_frequencies"],
1111
+ document_lengths=payload["document_lengths"],
1112
+ average_document_length=payload["average_document_length"],
1113
+ k1=payload["k1"],
1114
+ b=payload["b"],
1115
+ )
1116
  except Exception as exc:
1117
  logger.warning(
1118
  "Failed to load BM25 index; falling back to dense-only retrieval. "
 
1122
  exc,
1123
  )
1124
  return None
1125
+ return index
 
 
 
 
 
 
 
1126
 
1127
 
1128
  def default_bm25_index_path(document_dict_path: str) -> str:
 
1130
  name = path.name
1131
  if name.startswith("document_dict_") and name.endswith(".pkl"):
1132
  source = name.removeprefix("document_dict_").removesuffix(".pkl")
1133
+ return str(path.with_name(f"bm25_index_{source}.json.gz"))
1134
+ return str(path.with_name("bm25_index.json.gz"))
1135
 
1136
 
1137
  def result_dedupe_key(result: SearchResult) -> str:
app/config.py CHANGED
@@ -45,7 +45,7 @@ if langsmith_tracing_enabled():
45
  VECTOR_DB_DIR = "data/chroma-db-all_sources"
46
  VECTOR_COLLECTION_NAME = "chroma-db-all_sources"
47
  DOCUMENT_DICT_PATH = f"{VECTOR_DB_DIR}/document_dict_all_sources.pkl"
48
- BM25_INDEX_PATH = f"{VECTOR_DB_DIR}/bm25_index_all_sources.pkl"
49
  CHROMA_SQLITE_PATH = f"{VECTOR_DB_DIR}/chroma.sqlite3"
50
  # The full bundle (all sources, incl. gated course content) lives in the
51
  # private repo; cold-starting from it needs an HF_TOKEN with read access. When
 
45
  VECTOR_DB_DIR = "data/chroma-db-all_sources"
46
  VECTOR_COLLECTION_NAME = "chroma-db-all_sources"
47
  DOCUMENT_DICT_PATH = f"{VECTOR_DB_DIR}/document_dict_all_sources.pkl"
48
+ BM25_INDEX_PATH = f"{VECTOR_DB_DIR}/bm25_index_all_sources.json.gz"
49
  CHROMA_SQLITE_PATH = f"{VECTOR_DB_DIR}/chroma.sqlite3"
50
  # The full bundle (all sources, incl. gated course content) lives in the
51
  # private repo; cold-starting from it needs an HF_TOKEN with read access. When
data/scraping_scripts/build_public_docs_bundle.py CHANGED
@@ -140,7 +140,7 @@ logger = logging.getLogger(__name__)
140
  VECTOR_DB_DIR = "chroma-db-all_sources"
141
  VECTOR_COLLECTION_NAME = "chroma-db-all_sources"
142
  DOCUMENT_DICT_FILE = "document_dict_all_sources.pkl"
143
- BM25_INDEX_FILE = "bm25_index_all_sources.pkl"
144
  KB_DIR_NAME = "kb"
145
  KB_ARCHIVE_NAME = "kb.tar.gz"
146
 
@@ -244,11 +244,11 @@ def stage_chroma(source_dir: Path, stage_dir: Path, *, vacuum: bool = True) -> d
244
  }
245
 
246
 
247
- def rebuild_retrieval_pkls(source_dir: Path, stage_dir: Path) -> dict:
248
  """Rebuild the BM25 index and document dict from a docs-only JSONL.
249
 
250
  Mirrors ``create_vector_stores.write_retrieval_artifacts`` so the public
251
- pkls are byte-for-byte what prod would write without the course rows.
252
  """
253
  jsonl_path = source_dir / Path(ALL_SOURCES_JSONL).name
254
  if not jsonl_path.exists():
@@ -707,7 +707,7 @@ def build_bundle(
707
 
708
  summary: dict = {"sources": sorted(DOC_SOURCE_KEYS)}
709
  summary.update(stage_chroma(source_dir, stage_dir, vacuum=vacuum))
710
- summary.update(rebuild_retrieval_pkls(source_dir, stage_dir))
711
  summary.update(stage_kb(source_dir, stage_dir))
712
  summary.update(publicize_wiki(stage_dir))
713
  write_dataset_card(stage_dir)
 
140
  VECTOR_DB_DIR = "chroma-db-all_sources"
141
  VECTOR_COLLECTION_NAME = "chroma-db-all_sources"
142
  DOCUMENT_DICT_FILE = "document_dict_all_sources.pkl"
143
+ BM25_INDEX_FILE = "bm25_index_all_sources.json.gz"
144
  KB_DIR_NAME = "kb"
145
  KB_ARCHIVE_NAME = "kb.tar.gz"
146
 
 
244
  }
245
 
246
 
247
+ def rebuild_retrieval_artifacts(source_dir: Path, stage_dir: Path) -> dict:
248
  """Rebuild the BM25 index and document dict from a docs-only JSONL.
249
 
250
  Mirrors ``create_vector_stores.write_retrieval_artifacts`` so the public
251
+ artifacts are byte-for-byte what prod would write without the course rows.
252
  """
253
  jsonl_path = source_dir / Path(ALL_SOURCES_JSONL).name
254
  if not jsonl_path.exists():
 
707
 
708
  summary: dict = {"sources": sorted(DOC_SOURCE_KEYS)}
709
  summary.update(stage_chroma(source_dir, stage_dir, vacuum=vacuum))
710
+ summary.update(rebuild_retrieval_artifacts(source_dir, stage_dir))
711
  summary.update(stage_kb(source_dir, stage_dir))
712
  summary.update(publicize_wiki(stage_dir))
713
  write_dataset_card(stage_dir)
data/scraping_scripts/source_registry.py CHANGED
@@ -485,7 +485,7 @@ def vector_store_source_configs() -> dict[str, dict[str, str]]:
485
  "input_file": str(config["output_file"]),
486
  "db_name": f"chroma-db-{source}",
487
  "document_dict_file": f"document_dict_{source}.pkl",
488
- "bm25_index_file": f"bm25_index_{source}.pkl",
489
  }
490
  for source, config in SOURCE_CONFIGS.items()
491
  }
@@ -493,6 +493,6 @@ def vector_store_source_configs() -> dict[str, dict[str, str]]:
493
  "input_file": ALL_SOURCES_JSONL,
494
  "db_name": "chroma-db-all_sources",
495
  "document_dict_file": "document_dict_all_sources.pkl",
496
- "bm25_index_file": "bm25_index_all_sources.pkl",
497
  }
498
  return configs
 
485
  "input_file": str(config["output_file"]),
486
  "db_name": f"chroma-db-{source}",
487
  "document_dict_file": f"document_dict_{source}.pkl",
488
+ "bm25_index_file": f"bm25_index_{source}.json.gz",
489
  }
490
  for source, config in SOURCE_CONFIGS.items()
491
  }
 
493
  "input_file": ALL_SOURCES_JSONL,
494
  "db_name": "chroma-db-all_sources",
495
  "document_dict_file": "document_dict_all_sources.pkl",
496
+ "bm25_index_file": "bm25_index_all_sources.json.gz",
497
  }
498
  return configs
tests/test_build_public_docs_bundle.py CHANGED
@@ -227,7 +227,7 @@ def test_stage_kb_prunes_non_allowlisted_wiki_framework_pages(tmp_path: Path) ->
227
  builder.audit_staged_kb(stage_dir)
228
 
229
 
230
- def test_rebuild_retrieval_pkls_drops_course_documents(tmp_path: Path) -> None:
231
  source_dir = tmp_path / "data"
232
  rows = [
233
  {
@@ -252,7 +252,7 @@ def test_rebuild_retrieval_pkls_drops_course_documents(tmp_path: Path) -> None:
252
  _write_jsonl(source_dir / "all_sources_data.jsonl", rows)
253
  (tmp_path / "stage" / builder.VECTOR_DB_DIR).mkdir(parents=True)
254
 
255
- summary = builder.rebuild_retrieval_pkls(source_dir, tmp_path / "stage")
256
  assert summary["documents"] == 1
257
 
258
  dict_path = tmp_path / "stage" / builder.VECTOR_DB_DIR / builder.DOCUMENT_DICT_FILE
 
227
  builder.audit_staged_kb(stage_dir)
228
 
229
 
230
+ def test_rebuild_retrieval_artifacts_drops_course_documents(tmp_path: Path) -> None:
231
  source_dir = tmp_path / "data"
232
  rows = [
233
  {
 
252
  _write_jsonl(source_dir / "all_sources_data.jsonl", rows)
253
  (tmp_path / "stage" / builder.VECTOR_DB_DIR).mkdir(parents=True)
254
 
255
+ summary = builder.rebuild_retrieval_artifacts(source_dir, tmp_path / "stage")
256
  assert summary["documents"] == 1
257
 
258
  dict_path = tmp_path / "stage" / builder.VECTOR_DB_DIR / builder.DOCUMENT_DICT_FILE
tests/test_chroma_rag.py CHANGED
@@ -115,14 +115,14 @@ After the example.
115
  count = write_retrieval_artifacts(
116
  config={
117
  "document_dict_file": "document_dict_test.pkl",
118
- "bm25_index_file": "bm25_index_test.pkl",
119
  },
120
  document_rows=document_rows,
121
  db_path=str(db_path),
122
  )
123
 
124
  document_dict_path = db_path / "document_dict_test.pkl"
125
- bm25_path = db_path / "bm25_index_test.pkl"
126
 
127
  self.assertGreaterEqual(count, 1)
128
  self.assertTrue(document_dict_path.exists())
 
115
  count = write_retrieval_artifacts(
116
  config={
117
  "document_dict_file": "document_dict_test.pkl",
118
+ "bm25_index_file": "bm25_index_test.json.gz",
119
  },
120
  document_rows=document_rows,
121
  db_path=str(db_path),
122
  )
123
 
124
  document_dict_path = db_path / "document_dict_test.pkl"
125
+ bm25_path = db_path / "bm25_index_test.json.gz"
126
 
127
  self.assertGreaterEqual(count, 1)
128
  self.assertTrue(document_dict_path.exists())
tests/test_config.py CHANGED
@@ -30,7 +30,7 @@ def _patched_bundle(tmp_path: Path) -> ExitStack:
30
  ("VECTOR_DB_DIR", db_dir),
31
  ("CHROMA_SQLITE_PATH", db_dir / "chroma.sqlite3"),
32
  ("DOCUMENT_DICT_PATH", db_dir / "document_dict.pkl"),
33
- ("BM25_INDEX_PATH", db_dir / "bm25.pkl"),
34
  ("KB_MANIFEST_PATH", kb_dir / "generated" / "corpus_manifest.jsonl"),
35
  ("KB_INDEX_PATH", kb_dir / "wiki" / "index.md"),
36
  ("KB_AGENTS_PATH", kb_dir / "AGENTS.md"),
@@ -47,7 +47,7 @@ def _write_bundle_files(tmp_path: Path, *, include_bm25: bool = True) -> None:
47
  (db_dir / "chroma.sqlite3").write_bytes(b"")
48
  (db_dir / "document_dict.pkl").write_bytes(b"")
49
  if include_bm25:
50
- (db_dir / "bm25.pkl").write_bytes(b"")
51
  (tmp_path / "kb" / "generated").mkdir(parents=True, exist_ok=True)
52
  (tmp_path / "kb" / "generated" / "corpus_manifest.jsonl").write_text(
53
  "", encoding="utf-8"
@@ -120,7 +120,7 @@ def test_complete_bundle_skips_download_and_caches_readiness(tmp_path: Path) ->
120
 
121
  # Once verified, later calls are flag-checks: no re-stat, no
122
  # download attempt even if files vanish mid-process.
123
- (tmp_path / "chroma" / "bm25.pkl").unlink()
124
  config.ensure_local_vector_db()
125
  assert snapshot_download.call_count == 0
126
 
 
30
  ("VECTOR_DB_DIR", db_dir),
31
  ("CHROMA_SQLITE_PATH", db_dir / "chroma.sqlite3"),
32
  ("DOCUMENT_DICT_PATH", db_dir / "document_dict.pkl"),
33
+ ("BM25_INDEX_PATH", db_dir / "bm25.json.gz"),
34
  ("KB_MANIFEST_PATH", kb_dir / "generated" / "corpus_manifest.jsonl"),
35
  ("KB_INDEX_PATH", kb_dir / "wiki" / "index.md"),
36
  ("KB_AGENTS_PATH", kb_dir / "AGENTS.md"),
 
47
  (db_dir / "chroma.sqlite3").write_bytes(b"")
48
  (db_dir / "document_dict.pkl").write_bytes(b"")
49
  if include_bm25:
50
+ (db_dir / "bm25.json.gz").write_bytes(b"")
51
  (tmp_path / "kb" / "generated").mkdir(parents=True, exist_ok=True)
52
  (tmp_path / "kb" / "generated" / "corpus_manifest.jsonl").write_text(
53
  "", encoding="utf-8"
 
120
 
121
  # Once verified, later calls are flag-checks: no re-stat, no
122
  # download attempt even if files vanish mid-process.
123
+ (tmp_path / "chroma" / "bm25.json.gz").unlink()
124
  config.ensure_local_vector_db()
125
  assert snapshot_download.call_count == 0
126
 
tests/test_create_vector_stores.py CHANGED
@@ -72,7 +72,7 @@ class ProcessSourceContentAwareReuseTestCase(unittest.TestCase):
72
  "input_file": f"data/{self.source}_data.jsonl",
73
  "db_name": self.db_name,
74
  "document_dict_file": f"document_dict_{self.source}.pkl",
75
- "bm25_index_file": f"bm25_index_{self.source}.pkl",
76
  }
77
  with open(config["input_file"], "w", encoding="utf-8") as handle:
78
  handle.write(
 
72
  "input_file": f"data/{self.source}_data.jsonl",
73
  "db_name": self.db_name,
74
  "document_dict_file": f"document_dict_{self.source}.pkl",
75
+ "bm25_index_file": f"bm25_index_{self.source}.json.gz",
76
  }
77
  with open(config["input_file"], "w", encoding="utf-8") as handle:
78
  handle.write(