Local files still cross a boundary
A retrieval service needs to recover its index after a restart. Saving a Python object can make that convenient, but the file format becomes part of the application's trust model. If an index artifact can be replaced or modified outside the trusted build process, loading it must not quietly grant it the authority of application code.
Python's serialization documentation explains the hazard: unpickling untrusted or tampered data can execute code. That is a property of the format, not evidence that this project suffered a compromise. The PolicyScout critique records pickle persistence as an earlier defect and JSON plus rebuilding as its replacement. The public revision inspected here contains the replacement; it does not provide a separately reviewable before-and-after commit history.
The artifact should supply document records. The application should supply the construction logic.
Persist records; reconstruct behavior
The current persistence methods write document chunks to chunks.jsonl and descriptive metadata to manifest.json. Loading delegates to the normal build path, which parses the records and fits the TF-IDF and BM25 structures. The inspected module contains no pickle import or call.
Earlier, according to the critique:
saved object → object deserializer → restored index
Inspected implementation:
chunk records → JSONL → build() → retrieval structures
manifest file → existence checkThe tradeoff is work at startup: each reload rebuilds the index. This review includes no timing benchmark. Reproducibility also depends on the corpus, configuration, and numerical-library versions; the same file format alone does not establish identical ranking across environments.
Read what the assertion proves
The published round-trip regression saves an index, checks for a manifest, checks that no *.pkl file was written, loads the result, and requires a nonempty search response. Its input fixture ingests the repository's sample corpus into a temporary directory. A reviewer can select this test in an installed checkout:
python -m pytest tests/test_pipeline.py \
-k test_index_save_load_roundtrip_no_pickle -vThose assertions cover a useful persistence round trip. Checking a filename suffix does not, by itself, prove the absence of object deserialization elsewhere. Nor does one successful query prove ranking equivalence. The repository's workflow configures tests on Python 3.10 and 3.12; this page reports source inspection, not a fresh CI run or a claim that every security check passed.
A manifest is only useful if checked
One gap is visible directly in load(): it requires the manifest file to exist, but never reads its contents. The written version, backend identifier, and chunk count therefore are not validated on reload. Calling this a validated manifest would overstate the implementation.
My next review checks would exercise malformed metadata, unsupported versions, mismatched counts, missing text fields, and bounded record sizes. I would also compare selected query results before and after reload and record the dependency versions. These are proposed extensions, not tests claimed to exist in this revision.
PROPERTY ESTABLISHED / WORK REMAINING
JSON reconstruction removes the inspected pickle-loading path. It does not authenticate the corpus, prevent altered documents from influencing retrieval, or make arbitrarily large inputs inexpensive. Python's JSON documentation explicitly discusses resource consumption when processing untrusted input. Access controls, input limits, and artifact provenance remain separate requirements.
The engineering result is a narrower persistence contract and a precise next question. That is more useful to a reviewer than treating a format change as a complete security boundary.