feat: ingest idempotency key and content dedup (#15)

[leo-aa88]

Closes #15.

Two layers so retries do not double-count clusters:

Request idempotency

Idempotency-Key on POST /v1/ingestions and the unversioned alias (batch enqueue and tail create). A repeat within INGEST_IDEMPOTENCY_TTL_SECONDS (default 86400) returns the original 202 and the same job id. Empty or oversized keys → 400. Expired keys start a new job.

Content dedup

Each new log row stores original_line_hash (SHA-256 of the raw line, distinct from the fingerprint) and scope (API key scope, else "default"). Missing source_ref is stored as "". Partial unique index ux_log_entries_dedup on (scope, source_ref, original_line_hash, timestamp). Persist uses INSERT … ON CONFLICT DO NOTHING; duplicates are skipped, not errors. Embeddings are written only for inserted rows.

Applies to file ingest, adapter ingest (including tail ticks), and push lines.

Schema

Alembic 0007_ingest_idempotency. Applied 0001–0006 are untouched. Query isolation by scope remains G8.

Tests

Unit tests cover hashing, ON CONFLICT flush, Idempotency-Key replay/expiry, and OpenAPI header docs. A skip-gated integration test asserts re-ingest of identical lines keeps row count at 1 when Postgres is available.

  

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cursor]

Code review — PR #43 (issue #15)

CI is green. Checked against issue #15 / G6: Idempotency-Key TTL replay, empty-key 400, unique-key race → existing job, SHA-256 original_line_hash (not fingerprint), partial unique index + ON CONFLICT DO NOTHING (compiled SQL targets ux_log_entries_dedup including the WHERE predicate), embeddings only for RETURNING inserts, Alembic 0007 only (0001–0006 untouched), empty source_ref stored as "", G8 query filtering not claimed, dual-mount /v1/ingestions and /ingestions. Upsert unit tests would fail if ON CONFLICT were dropped.

must-fix

None

should-fix

• tests/unit/test_ingest_idempotency.py:118-126 and :166-172 (TestCreateIngestionIdempotency.test_same_key_returns_original_worker_job_id, test_same_key_on_unversioned_alias): capture_add assigns the same job_id to every WorkerJob. If _replay_if_active / _bind_idempotency_key were removed, both POSTs would still return that UUID and the assertions would pass. Empty/oversized-key 400 tests would still fail, but the core “same key → same job” contract would not. Give each WorkerJob a fresh uuid.uuid4() (as test_different_keys_create_two_jobs already does) and assert the second response equals the first id, not a fixture constant.

• src/api/routes/ingestions.py:241-245 (_ingest_payload): callback_meta (including the API key’s scope) is merged only when callback_url is set. src/worker/runner.py:61 / :135 and src/core/ingestion/tail.py:241 read payload/meta["scope"], and POST /v1/ingestions/lines correctly uses _scope_from_request. Batch enqueue and tail create therefore persist scope="default" for a scoped principal unless a webhook URL is also present. Concrete failure: two keys with different scopes POST /v1/ingestions the same file (no callback_url) share one unique-index namespace and the second persist is skipped; the same keys on /lines keep separate rows. README claims scope is the principal’s when present. Always copy scope onto the worker payload / tail metadata_json.

nice-to-have

• tests/unit/test_ingest_dedup.py:96-101: compiled SQL is not asserted to include timestamp in the conflict target or the partial-index WHERE. Dropping index_where would still pass these tests and then fail at persist on Postgres (no unique or exclusion constraint matching ON CONFLICT).
• No HTTP coverage for tail-mode Idempotency-Key replay (mode=tail / ingestion_job_id); batch is covered.
• tests/integration/test_ingest_dedup.py asserts row count == 1 only and is skip-gated without Postgres. Issue feat(G6): ingest idempotency & content dedup #15 AC also asked for stable cluster counts / change-ratios; row uniqueness makes that true by construction, so this is optional.

Verdict

must-fix count: 0
should-fix count: 2

[cursor]

Code review — PR #43 (issue #15), round 2

Follow-up f8791ea vs origin/main. CI is green. Re-checked G6: Idempotency-Key TTL replay, empty-key 400, unique-key race → existing job, SHA-256 original_line_hash (not fingerprint), partial unique index + ON CONFLICT DO NOTHING, embeddings only for RETURNING inserts, Alembic 0007 only, empty source_ref stored as "".

Previous should-fix status

• item 1: fixed. tests/unit/test_ingest_idempotency.py (test_same_key_returns_original_worker_job_id, test_same_key_on_unversioned_alias): capture_add now assigns uuid.uuid4() per WorkerJob and asserts first.json()["worker_job_id"] == second.json()["worker_job_id"] (plus 202 on both). If _replay_if_active and _bind_idempotency_key were removed, the second POST would enqueue a distinct UUID and the equality assertion would fail. The old shared-fixture job_id would still have passed that case.
• item 2: fixed. src/api/routes/ingestions.py _ingest_payload always payload.update(callback_meta) before optionally setting callback_url. tests/unit/test_api.py test_scope_on_payload_without_callback_url authenticates a principal with scope="incident:A", POSTs without callback_url, and asserts payload_json["scope"] == "incident:A" and "callback_url" not in payload. Restoring the old if request.callback_url: gate would KeyError/fail that assertion. test_omitted_callback_url_is_not_on_payload now also requires scope == "default".

must-fix

None

should-fix

None

nice-to-have

None

Verdict

must-fix count: 0
should-fix count: 0