Skip to content

Commit d31db86

Browse files
committed
fix(ingest): scope document uniqueness to the ingestion space so each config gets its own chunks
1 parent 4cd409e commit d31db86

4 files changed

Lines changed: 82 additions & 3 deletions

File tree

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
"""scope document uniqueness to the ingestion space
2+
3+
A document was unique per (tenant_id, sha256), so re-uploading the same PDF
4+
under a different ingestion space returned the existing row and skipped
5+
ingestion entirely. Config A's space therefore stayed empty while config B's
6+
filled — and the A-vs-B comparison the benchmark exists to make was impossible.
7+
8+
Chunks were already keyed per space. This brings documents in line: the same
9+
file ingested under two chunking strategies is two ingestion records, because
10+
it produces two independent sets of chunks and embeddings.
11+
12+
The cost is a duplicated blob per space. At demo scale that is a few megabytes,
13+
and the alternative — tracking ingestion state per (document, space) in a
14+
separate table — is the right answer only once storage actually matters.
15+
16+
Revision ID: 0002
17+
Revises: 0001
18+
Create Date: 2026-07-26
19+
"""
20+
21+
from __future__ import annotations
22+
23+
from alembic import op
24+
25+
revision = "0002"
26+
down_revision = "0001"
27+
branch_labels = None
28+
depends_on = None
29+
30+
31+
def upgrade() -> None:
32+
op.drop_constraint("uq_documents_tenant_sha", "documents", type_="unique")
33+
op.create_unique_constraint(
34+
"uq_documents_tenant_sha_space", "documents", ["tenant_id", "sha256", "space"]
35+
)
36+
37+
38+
def downgrade() -> None:
39+
# Collapsing back to one row per file requires deleting every duplicate
40+
# first; doing that silently would destroy ingested chunks, so this is left
41+
# to a deliberate manual step.
42+
op.drop_constraint("uq_documents_tenant_sha_space", "documents", type_="unique")
43+
op.create_unique_constraint("uq_documents_tenant_sha", "documents", ["tenant_id", "sha256"])

app/db/models.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,13 @@ class Document(Base):
120120
pages = relationship("Page", back_populates="document", cascade="all, delete-orphan")
121121
chunks = relationship("Chunk", back_populates="document", cascade="all, delete-orphan")
122122

123-
__table_args__ = (UniqueConstraint("tenant_id", "sha256", name="uq_documents_tenant_sha"),)
123+
# Scoped to `space`: the same file ingested under two chunking strategies is
124+
# two ingestion records, because it yields two independent sets of chunks
125+
# and embeddings. Keying on the hash alone made the second ingest a no-op
126+
# and left config A's space empty.
127+
__table_args__ = (
128+
UniqueConstraint("tenant_id", "sha256", "space", name="uq_documents_tenant_sha_space"),
129+
)
124130

125131

126132
class DocumentBlob(Base):

app/ingest/pipeline.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,17 @@ async def create_document(
115115
paying to parse and embed the same corpus twice.
116116
"""
117117
digest = content_sha256(data)
118+
space = space_for(config)
119+
120+
# Scoped to the space. Matching on the hash alone would return the copy
121+
# ingested under a different chunking strategy and skip the work, leaving
122+
# this space with no chunks at all.
118123
existing = await session.scalar(
119-
select(Document).where(Document.tenant_id == tenant_id, Document.sha256 == digest)
124+
select(Document).where(
125+
Document.tenant_id == tenant_id,
126+
Document.sha256 == digest,
127+
Document.space == space,
128+
)
120129
)
121130
if existing is not None:
122131
return existing, False
@@ -128,7 +137,7 @@ async def create_document(
128137
size_bytes=len(data),
129138
sha256=digest,
130139
status=DocumentStatus.UPLOADED,
131-
space=space_for(config),
140+
space=space,
132141
meta={"chunk_strategy": config.chunk_strategy.value, "config": config.name},
133142
)
134143
session.add(doc)

tests/test_schema_contract.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,24 @@ def test_enum_members_serialise_as_their_lowercase_value():
7676
assert DocumentStatus.READY == "ready"
7777
assert DocumentStatus.READY.value == "ready"
7878
assert ExtractionMethod.VISION == "vision"
79+
80+
81+
def test_documents_are_unique_per_ingestion_space():
82+
"""Keyed on (tenant, sha256) alone, re-ingesting the same PDF under a
83+
different chunking strategy returned the existing row and did nothing — so
84+
config A's space stayed empty and the A-vs-B comparison, which is the whole
85+
point of the benchmark, could not be made.
86+
87+
Chunks were already per-space; documents have to agree.
88+
"""
89+
constraints = {
90+
c.name: sorted(col.name for col in c.columns)
91+
for c in Document.__table__.constraints
92+
if c.__class__.__name__ == "UniqueConstraint"
93+
}
94+
assert "uq_documents_tenant_sha_space" in constraints, constraints
95+
assert constraints["uq_documents_tenant_sha_space"] == ["sha256", "space", "tenant_id"]
96+
97+
migrations = sorted(MIGRATION.parent.glob("*.py"))
98+
source = "\n".join(p.read_text() for p in migrations)
99+
assert "uq_documents_tenant_sha_space" in source, "the ORM constraint has no matching migration"

0 commit comments

Comments
 (0)