fix(llm): empty-text items are stamped as final and count toward docket completion (refs #615)
Treating a blank load() as unindexable defeated the feature it was meant to protect: four of the eleven historical dockets hold 1-2 comments with no text at all, so none of them could ever be marked complete and ~130k items would be re-listed on every run. An item with no text is a finished state, not a failure. Stamp it into index_state with content_hash='' / chunk_count=0 and its fingerprint — the empty hash cannot collide with a real sha256 — so the next run fingerprint-skips it without a load, and it no longer blocks its docket's completion. Text appearing later changes the fingerprint and the item is embedded normally. Only a ref that loaded *with* text and chunked to nothing stays unindexable and blocks index_docket_state; the embed/write path already aborts the whole run on error. The index_state upsert both callers need is now _record_state().
This commit is contained in:
@@ -101,6 +101,32 @@ def _delete_old_chunks(engine: Engine, item_key: str, collection: str) -> None:
|
||||
log.debug("langchain_pg_embedding not present yet; skipping delete")
|
||||
|
||||
|
||||
def _record_state(
|
||||
engine: Engine, key: str, collection: str, h: str, n: int, fingerprint: str
|
||||
) -> None:
|
||||
"""Upsert this item's row in ``index_state``.
|
||||
|
||||
``h == "" and n == 0`` is the *stamped-final* form: an item with no
|
||||
text at all. It is a finished state, not a failure — no chunks exist
|
||||
to delete or embed — and the empty hash can never collide with a real
|
||||
``content_hash`` (a sha256 hexdigest), so the next run recognises it
|
||||
by fingerprint alone and never loads it again. If text does appear
|
||||
later the fingerprint changes and the item is embedded normally.
|
||||
"""
|
||||
with engine.begin() as conn:
|
||||
conn.execute(
|
||||
text(
|
||||
"INSERT INTO index_state "
|
||||
"(item_key, collection, content_hash, chunk_count, fingerprint) "
|
||||
"VALUES (:k, :c, :h, :n, :f) "
|
||||
"ON CONFLICT (item_key, collection) DO UPDATE SET "
|
||||
"content_hash = :h, chunk_count = :n, fingerprint = :f, "
|
||||
"indexed_at = now()"
|
||||
),
|
||||
{"k": key, "c": collection, "h": h, "n": n, "f": fingerprint},
|
||||
)
|
||||
|
||||
|
||||
def index_refs(
|
||||
refs: Iterable[DocRef],
|
||||
*,
|
||||
@@ -117,14 +143,19 @@ def index_refs(
|
||||
Per ref: (1) fingerprint equal to the stored one → skip without
|
||||
loading; (2) load, hash the text; hash equal → record the new
|
||||
fingerprint, skip embedding; (3) chunk, locate PDF pages, embed,
|
||||
write. PDFs are opened only on path (3). After the loop, a docket in
|
||||
*sealed* is recorded in ``index_docket_state`` — so the next run does
|
||||
not list it at all — only when every one of its refs was accounted
|
||||
for: a ref that loaded blank or produced no chunks is *unindexable*
|
||||
and blocks completion, because a seal must never claim coverage this
|
||||
run did not achieve. *mark_complete* False (a ``--limit`` run is
|
||||
never complete) blocks it too. Pass *engine* to reuse a caller's
|
||||
engine instead of building a second one.
|
||||
write. PDFs are opened only on path (3). A ref that loads blank is
|
||||
*stamped final* (see :func:`_record_state`) rather than treated as a
|
||||
failure — plenty of real comments carry no text at all.
|
||||
|
||||
After the loop, a docket in *sealed* is recorded in
|
||||
``index_docket_state`` — so the next run does not list it at all —
|
||||
only when every one of its refs was accounted for. A ref that loaded
|
||||
*with* text but chunked to nothing is *unindexable* and blocks
|
||||
completion, because a seal must never claim coverage this run did not
|
||||
achieve (an exception on the embed/write path aborts the run and so
|
||||
blocks it too). *mark_complete* False (a ``--limit`` run is never
|
||||
complete) blocks it as well. Pass *engine* to reuse a caller's engine
|
||||
instead of building a second one.
|
||||
"""
|
||||
engine = engine if engine is not None else _engine(cfg)
|
||||
migrate(engine)
|
||||
@@ -156,7 +187,8 @@ def index_refs(
|
||||
continue
|
||||
doc = ref.load()
|
||||
if doc is None or not doc.text.strip():
|
||||
_unindexable(ref)
|
||||
_record_state(engine, ref.key, collection, "", 0, ref.fingerprint)
|
||||
stats["skipped"] += 1
|
||||
continue
|
||||
h = content_hash(doc.text)
|
||||
if not force and prev_hash == h:
|
||||
@@ -182,24 +214,7 @@ def index_refs(
|
||||
metadatas=[c.metadata for c in chunks],
|
||||
ids=[c.id for c in chunks],
|
||||
)
|
||||
with engine.begin() as conn:
|
||||
conn.execute(
|
||||
text(
|
||||
"INSERT INTO index_state "
|
||||
"(item_key, collection, content_hash, chunk_count, fingerprint) "
|
||||
"VALUES (:k, :c, :h, :n, :f) "
|
||||
"ON CONFLICT (item_key, collection) DO UPDATE SET "
|
||||
"content_hash = :h, chunk_count = :n, fingerprint = :f, "
|
||||
"indexed_at = now()"
|
||||
),
|
||||
{
|
||||
"k": ref.key,
|
||||
"c": collection,
|
||||
"h": h,
|
||||
"n": len(chunks),
|
||||
"f": ref.fingerprint,
|
||||
},
|
||||
)
|
||||
_record_state(engine, ref.key, collection, h, len(chunks), ref.fingerprint)
|
||||
stats["indexed"] += 1
|
||||
stats["chunks"] += len(chunks)
|
||||
# needs 100+ docs in one run to hit this line
|
||||
|
||||
@@ -32,6 +32,13 @@ def _ref(fp="fp1", docket="D", loads=None, doc=DOC):
|
||||
)
|
||||
|
||||
|
||||
def _maybe_chunk_doc(fn):
|
||||
"""Patch llm.index.chunk_doc only when a test asks for it."""
|
||||
from contextlib import nullcontext
|
||||
|
||||
return nullcontext() if fn is None else patch("llm.index.chunk_doc", side_effect=fn)
|
||||
|
||||
|
||||
def _run(
|
||||
refs,
|
||||
state_rows,
|
||||
@@ -40,7 +47,7 @@ def _run(
|
||||
sealed=None,
|
||||
mark_complete=True,
|
||||
complete_rows=(),
|
||||
chunks_for=lambda d, c: c,
|
||||
chunk_doc=None,
|
||||
):
|
||||
store = MagicMock()
|
||||
engine = MagicMock()
|
||||
@@ -63,8 +70,9 @@ def _run(
|
||||
patch("llm.index.vectorstore", return_value=store),
|
||||
patch("llm.index.embed_texts", return_value=[[0.0] * 3]),
|
||||
patch("llm.index.ensure_hnsw"),
|
||||
patch("llm.index.enrich_pdf_pages", side_effect=chunks_for) as enrich,
|
||||
patch("llm.index.enrich_pdf_pages", side_effect=lambda d, c: c) as enrich,
|
||||
patch("llm.index.HostPool") as MockPool,
|
||||
_maybe_chunk_doc(chunk_doc),
|
||||
):
|
||||
MockPool.return_value.check.return_value = ["http://h1:11434"]
|
||||
stats = index_refs(
|
||||
@@ -152,30 +160,59 @@ def test_not_marked_when_mark_complete_false_or_unsealed():
|
||||
assert stats["docket_complete"] == 0
|
||||
|
||||
|
||||
def test_sealed_docket_not_marked_when_a_ref_is_unindexable():
|
||||
"""A docket with an item that has no indexable text is not complete."""
|
||||
def _state_inserts(conn):
|
||||
return [
|
||||
c
|
||||
for c in conn.execute.call_args_list
|
||||
if "INSERT INTO index_state" in str(c.args[0])
|
||||
]
|
||||
|
||||
|
||||
def _docket_inserts(conn):
|
||||
return [
|
||||
c
|
||||
for c in conn.execute.call_args_list
|
||||
if "INSERT INTO index_docket_state" in str(c.args[0])
|
||||
]
|
||||
|
||||
|
||||
def test_empty_text_ref_is_stamped_final_and_counts_toward_completion():
|
||||
"""No text is a finished state, not a failure: stamp it so the next
|
||||
run fingerprint-skips it, and let the docket still seal complete."""
|
||||
blank = DocRef(
|
||||
key="K9", collection="comments", docket="D", fingerprint="x", load=lambda: None
|
||||
)
|
||||
stats, _, conn, _ = _run(
|
||||
[_ref("fp1"), blank], [], sealed={"D": "2026-10-20T00:00:00Z"}
|
||||
stats, store, conn, _ = _run([blank], [], sealed={"D": "s"})
|
||||
assert stats["skipped"] == 1 and stats["indexed"] == 0
|
||||
store.add_embeddings.assert_not_called()
|
||||
ins = _state_inserts(conn)
|
||||
assert len(ins) == 1
|
||||
assert ins[0].args[1] == {"k": "K9", "c": "comments", "h": "", "n": 0, "f": "x"}
|
||||
assert stats["docket_complete"] == 1
|
||||
assert _docket_inserts(conn)
|
||||
|
||||
|
||||
def test_stamped_empty_row_is_fingerprint_skipped_on_the_next_run():
|
||||
loads = []
|
||||
blank = DocRef(
|
||||
key="K9",
|
||||
collection="comments",
|
||||
docket="D",
|
||||
fingerprint="x",
|
||||
load=lambda: loads.append("K9"),
|
||||
)
|
||||
assert stats["skipped"] == 1 and stats["indexed"] == 1
|
||||
assert stats["docket_complete"] == 0
|
||||
assert not [
|
||||
c
|
||||
for c in conn.execute.call_args_list
|
||||
if "INSERT INTO index_docket_state" in str(c.args[0])
|
||||
]
|
||||
stats, _, conn, _ = _run([blank], [("K9", "", "x")])
|
||||
assert stats["fingerprint_skipped"] == 1 and stats["skipped"] == 0
|
||||
assert loads == [] # stamped-empty rows are never re-loaded
|
||||
assert _state_inserts(conn) == []
|
||||
|
||||
|
||||
def test_sealed_docket_not_marked_when_a_ref_yields_no_chunks():
|
||||
"""Text that chunks to nothing is a real failure — it blocks the seal
|
||||
and is not stamped."""
|
||||
stats, _, conn, _ = _run(
|
||||
[_ref("fp1")], [], sealed={"D": "s"}, chunks_for=lambda d, c: []
|
||||
[_ref("fp1")], [], sealed={"D": "s"}, chunk_doc=lambda d: []
|
||||
)
|
||||
assert stats["skipped"] == 1 and stats["docket_complete"] == 0
|
||||
assert not [
|
||||
c
|
||||
for c in conn.execute.call_args_list
|
||||
if "INSERT INTO index_docket_state" in str(c.args[0])
|
||||
]
|
||||
assert _state_inserts(conn) == []
|
||||
assert _docket_inserts(conn) == []
|
||||
|
||||
Reference in New Issue
Block a user