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:
kert
2026-09-08 15:52:34 -04:00
parent 6ca54a6da9
commit ed55a8f883
2 changed files with 98 additions and 46 deletions

View File

@@ -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

View File

@@ -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) == []