Skip to content

fix: reject duplicate document source bytes - #459

Merged
suguanYang merged 1 commit into
mainfrom
fix/wangbinqi/source-content-deduplication
Oct 10, 2026
Merged

suguanYang merged 1 commit into
mainfrom
fix/wangbinqi/source-content-deduplication

Conversation

@suguanYang

Copy link
Copy Markdown
Contributor

Summary

We replace filename-based ingestion rejection with a SHA-256 check of the downloaded original bytes. Different bytes under the same filename are accepted; identical bytes fail the job with ALREADY_EXISTS before parsing or billing, including unchanged document_id updates.

  • Scope admission to one user and namespace, comparing current active document revisions and in-progress job reservations. Failed jobs, archived or superseded revisions, and historical sources without fingerprints do not block admission.
  • Persist the original hash and byte size in job metadata. Serialize concurrent claims with an advisory transaction lock and check publication plus reservations in one statement snapshot.
  • Add an idempotent partial user/hash index migration; preserve the separate demo corpus admission policy. Update contracts for exports, billing, webhook outbox events, and controlled concurrency.

Related task: reported duplicate-upload exception.

Verification

  • uv run --no-sync --all-packages pytest apps/worker/tests/contract/test_parse_task_contract.py apps/worker/tests/contract/test_source_content_admission_contract.py -q: 33 passed after rebasing onto current main.
  • uv run --no-sync --all-packages pytest apps/api/tests/contract/test_job_creation_contract.py apps/api/tests/migrations/test_schema_contract.py -q: 65 passed, including all 12 index-definition migration scenarios, on the rebased branch.
  • Ruff across apps and packages, Pyright across API/worker/shared production paths plus changed API contracts and the migration, and git diff --check passed.
  • Real PostgreSQL ablations: baseline 14/14 passed; removing persistence, locking, in-progress reservations, or the shared statement snapshot produced the expected contract failures. Replacing the hash with file size falsely rejected different equal-size files.
  • A local 5,000-document query probe measured 0.093 ms with the hash index versus 37.883 ms without it; 2,000 matching hashes in other namespaces raised execution to 6.948 ms. These measure SQL execution only and are not a five-million-document production benchmark.

Detailed review and ablation report. URL transport and external webhook delivery were not exercised; webhook outbox creation and dispatch requests were verified locally.

Deployment Notes

  • Apply migration 5e6f7a8b9c0d, roll out workers, then remove the API filename gate. Older workers do not reserve content hashes; full enforcement requires the worker rollout to finish.
  • The index is created concurrently for ordinary Alembic upgrades, with a transactional path for caller-owned connections. There is no historical fingerprint backfill. No new environment variables or queue configuration.
  • Duplicate rejection now occurs through the job failure contract after upload/download. Exact byte comparison permits re-exported files with changed metadata and rejects unchanged sources even when parsing options change.
  • Rolling back API/worker code restores filename-based admission. Stored fingerprint metadata and the index can remain until migration downgrade.

Checklist

  • Tests were added or updated when behavior changed
  • Public docs, examples, or OpenAPI contracts were updated when needed
  • Database migrations are idempotent and safe to deploy
  • Logs, errors, and validation paths avoid leaking secrets or user data
  • The pull request description explains any breaking or user-visible change

from alembic import op
from sqlalchemy import text

revision: str = "5e6f7a8b9c0d"
from sqlalchemy import text

revision: str = "5e6f7a8b9c0d"
down_revision: str = "4d5e6f7a8b9c"

revision: str = "5e6f7a8b9c0d"
down_revision: str = "4d5e6f7a8b9c"
branch_labels: Sequence[str] | None = None
revision: str = "5e6f7a8b9c0d"
down_revision: str = "4d5e6f7a8b9c"
branch_labels: Sequence[str] | None = None
depends_on: Sequence[str] | None = None
@suguanYang
suguanYang merged commit d2900d2 into main Oct 10, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants