Skip to content

Commit a36f6ba

Browse files
committed
engine: idempotent ingest + self-diagnosing not-found (HAL-323)
Two-part fix for flaky bulk ingestion and the bench's duplicate uploads. Idempotent ingest (root cause of "same doc ingested up to 6x"): - New `idempotency_key` column on documents + partial unique index on (org_id, idempotency_key) — migration 0007. NULL key = no dedup (preserves current behavior for callers that don't opt in). - All three ingest handlers (standalone/local in internal/api, multi-tenant REST in internal/handler, Connect RPC in internal/connecthandler) honor the `Idempotency-Key` header the SDK already sends: a repeat ingest returns the original document instead of creating a duplicate, and the concurrent same-key race is resolved by dropping the orphan source and returning the winner. The SDK retry-on-transient (which re-POSTs after a reset that landed AFTER the server committed) is now a no-op. - db: ErrConflict sentinel mapped from SQLSTATE 23505; GetDocumentByIdempotencyKey. Self-diagnosing not-found (resolves the pathless "object not found" mystery): - Every storage backend's ErrNotFound now carries the resolved key/path (local Get+Delete, s3 Get+Delete, gcs Get+Delete) so a failure can always be attributed to a code path — pathless rows were pre-path-wrap leftovers. - local.Get: the open-after-stat result is wrapped as retryable ErrNotFound (previously a raw error getSourceWithRetry would NOT retry), and a Windows-only bounded retry rides through the Defender scan window that transiently hides a freshly-written file. Docs: OpenAPI documents the Idempotency-Key header; README documents the mandatory Windows Defender exclusion for the storage root. Verified: go build/vet clean; storage + db (live-Postgres) tests pass; and an end-to-end double-POST with one Idempotency-Key returns one document_id while a keyless control gets its own.
1 parent ebb9c10 commit a36f6ba

14 files changed

Lines changed: 446 additions & 51 deletions

README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,15 @@ llm:
164164
165165
> **Anthropic-compatible gateways (GLM/Z.ai):** `base_url` **must include `/v1`** — the client posts to `${base_url}/messages`.
166166

167+
> **Windows + local storage:** Windows Defender real-time protection scans
168+
> each freshly-written file and briefly hides it from `os.Stat`/`os.Open`,
169+
> which under heavy concurrent ingestion can surface as transient
170+
> `object not found` errors. The local backend rides through this window with
171+
> a short internal retry, but for large bulk loads **add a Defender exclusion
172+
> for your storage root** (`local.root` / `VLE_STORAGE_LOCAL_ROOT`):
173+
> `Add-MpPreference -ExclusionPath "C:\path\to\data\documents"`. Linux has no
174+
> such scan-hold and needs no exclusion.
175+
167176
### Supported formats
168177

169178
PDF (positioned text + tables via [`pdftable`](https://github.com/hallelx2/pdftable)) · Markdown · HTML · DOCX · Text.

internal/api/server.go

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,28 @@ func (d Deps) handleIngestDocument(w http.ResponseWriter, r *http.Request) {
231231

232232
docID := ingest.NewDocumentID()
233233

234+
// Idempotent ingest: if the caller supplied an Idempotency-Key and a
235+
// document already exists under it (for the standalone org), return that
236+
// document instead of creating a duplicate. A client — or the SDK's own
237+
// transport retry-on-transient — that re-POSTs the same upload after a
238+
// reset that landed AFTER the server committed the row is then a no-op.
239+
// This is the standalone/local-mode half of the HAL-323 duplicate-upload
240+
// fix (the multi-tenant handler in internal/handler mirrors it).
241+
idemKey := r.Header.Get("Idempotency-Key")
242+
if idemKey != "" {
243+
if existing, err := d.DB.GetDocumentByIdempotencyKey(ctx, standaloneOrgID, idemKey); err == nil {
244+
writeJSON(w, http.StatusAccepted, map[string]any{
245+
"document_id": existing.ID,
246+
"status": string(existing.Status),
247+
})
248+
return
249+
} else if !errors.Is(err, db.ErrNotFound) {
250+
d.Logger.Error("ingest: idempotency lookup failed", "err", err)
251+
writeErr(w, http.StatusInternalServerError, "db read failed")
252+
return
253+
}
254+
}
255+
234256
var (
235257
filename string
236258
contentType string
@@ -300,14 +322,27 @@ func (d Deps) handleIngestDocument(w http.ResponseWriter, r *http.Request) {
300322
}
301323

302324
if err := d.DB.NewDocument(ctx, db.Document{
303-
ID: docID,
304-
OrgID: standaloneOrgID,
305-
Title: title,
306-
ContentType: contentType,
307-
SourceRef: key,
308-
Status: db.StatusPending,
309-
ByteSize: size,
325+
ID: docID,
326+
OrgID: standaloneOrgID,
327+
Title: title,
328+
ContentType: contentType,
329+
SourceRef: key,
330+
Status: db.StatusPending,
331+
ByteSize: size,
332+
IdempotencyKey: idemKey,
310333
}); err != nil {
334+
// A concurrent same-key ingest won the race: drop the orphan source we
335+
// just wrote and return the winner instead of a 500.
336+
if idemKey != "" && errors.Is(err, db.ErrConflict) {
337+
_ = d.Storage.Delete(ctx, key)
338+
if existing, lookupErr := d.DB.GetDocumentByIdempotencyKey(ctx, standaloneOrgID, idemKey); lookupErr == nil {
339+
writeJSON(w, http.StatusAccepted, map[string]any{
340+
"document_id": existing.ID,
341+
"status": string(existing.Status),
342+
})
343+
return
344+
}
345+
}
311346
d.Logger.Error("ingest: db insert failed", "err", err)
312347
writeErr(w, http.StatusInternalServerError, "db write failed")
313348
return

internal/connecthandler/documents.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,23 @@ func (s *DocumentsService) CreateDocument(
8282
return nil, connect.NewError(connect.CodeInvalidArgument, errors.New("content is required"))
8383
}
8484

85+
// Idempotent ingest: a repeat CreateDocument carrying the same
86+
// Idempotency-Key returns the original document rather than a duplicate
87+
// (mirrors the REST path; closes the HAL-323 duplicate-upload bug for SDK
88+
// callers that retry the RPC).
89+
idemKey := req.Header().Get("Idempotency-Key")
90+
if idemKey != "" {
91+
if existing, err := s.db.GetDocumentByIdempotencyKey(ctx, orgID, idemKey); err == nil {
92+
return connect.NewResponse(&v1.CreateDocumentResponse{
93+
DocumentId: string(existing.ID),
94+
Status: string(existing.Status),
95+
}), nil
96+
} else if !errors.Is(err, db.ErrNotFound) {
97+
s.logger.Error("ingest: idempotency lookup failed", "err", err)
98+
return nil, connect.NewError(connect.CodeInternal, errors.New("db read failed"))
99+
}
100+
}
101+
85102
docID := ingest.NewDocumentID()
86103
contentType := msg.ContentType
87104
if contentType == "" {
@@ -105,15 +122,27 @@ func (s *DocumentsService) CreateDocument(
105122
}
106123

107124
if err := s.db.NewDocument(ctx, db.Document{
108-
ID: docID,
109-
OrgID: orgID,
110-
StoreID: storeIDFromConnect(req),
111-
Title: title,
112-
ContentType: contentType,
113-
SourceRef: key,
114-
Status: db.StatusPending,
115-
ByteSize: size,
125+
ID: docID,
126+
OrgID: orgID,
127+
StoreID: storeIDFromConnect(req),
128+
Title: title,
129+
ContentType: contentType,
130+
SourceRef: key,
131+
Status: db.StatusPending,
132+
ByteSize: size,
133+
IdempotencyKey: idemKey,
116134
}); err != nil {
135+
// Concurrent same-key insert won the race: drop the orphan source and
136+
// return the winner instead of erroring.
137+
if idemKey != "" && errors.Is(err, db.ErrConflict) {
138+
_ = s.storage.Delete(ctx, key)
139+
if existing, lookupErr := s.db.GetDocumentByIdempotencyKey(ctx, orgID, idemKey); lookupErr == nil {
140+
return connect.NewResponse(&v1.CreateDocumentResponse{
141+
DocumentId: string(existing.ID),
142+
Status: string(existing.Status),
143+
}), nil
144+
}
145+
}
117146
s.logger.Error("ingest: db insert failed", "err", err)
118147
return nil, connect.NewError(connect.CodeInternal, errors.New("db write failed"))
119148
}

internal/handler/documents.go

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,25 @@ func (h *DocumentsHandler) HandleIngestDocument(w http.ResponseWriter, r *http.R
158158
ctx := r.Context()
159159
docID := ingest.NewDocumentID()
160160

161+
// Idempotent ingest: if the client supplied an Idempotency-Key and we
162+
// already have a document for this org under that key, return it instead
163+
// of creating a duplicate. This makes a client/transport retry of the
164+
// ingest POST (the HAL-323 duplicate-upload bug) a no-op.
165+
idemKey := r.Header.Get("Idempotency-Key")
166+
if idemKey != "" {
167+
if existing, err := h.db.GetDocumentByIdempotencyKey(ctx, orgID, idemKey); err == nil {
168+
writeJSON(w, http.StatusAccepted, map[string]any{
169+
"document_id": existing.ID,
170+
"status": string(existing.Status),
171+
})
172+
return
173+
} else if !errors.Is(err, db.ErrNotFound) {
174+
h.logger.Error("ingest: idempotency lookup failed", "err", err)
175+
writeErr(w, http.StatusInternalServerError, "db read failed")
176+
return
177+
}
178+
}
179+
161180
var (
162181
filename string
163182
contentType string
@@ -227,15 +246,29 @@ func (h *DocumentsHandler) HandleIngestDocument(w http.ResponseWriter, r *http.R
227246
}
228247

229248
if err := h.db.NewDocument(ctx, db.Document{
230-
ID: docID,
231-
OrgID: orgID,
232-
StoreID: storeID(r),
233-
Title: title,
234-
ContentType: contentType,
235-
SourceRef: key,
236-
Status: db.StatusPending,
237-
ByteSize: size,
249+
ID: docID,
250+
OrgID: orgID,
251+
StoreID: storeID(r),
252+
Title: title,
253+
ContentType: contentType,
254+
SourceRef: key,
255+
Status: db.StatusPending,
256+
ByteSize: size,
257+
IdempotencyKey: idemKey,
238258
}); err != nil {
259+
// A concurrent request with the same Idempotency-Key won the race and
260+
// inserted first. Drop the orphan source we just wrote and return the
261+
// winner's document rather than a 500.
262+
if idemKey != "" && errors.Is(err, db.ErrConflict) {
263+
_ = h.storage.Delete(ctx, key)
264+
if existing, lookupErr := h.db.GetDocumentByIdempotencyKey(ctx, orgID, idemKey); lookupErr == nil {
265+
writeJSON(w, http.StatusAccepted, map[string]any{
266+
"document_id": existing.ID,
267+
"status": string(existing.Status),
268+
})
269+
return
270+
}
271+
}
239272
h.logger.Error("ingest: db insert failed", "err", err)
240273
writeErr(w, http.StatusInternalServerError, "db write failed")
241274
return

openapi.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,21 @@ paths:
113113
(field: "file") or JSON body with content. Returns immediately
114114
with a document_id in "pending" state. The ingest pipeline runs
115115
asynchronously: parse → build tree → summarize sections.
116+
117+
Pass an `Idempotency-Key` header to make ingestion idempotent: a
118+
repeated request with the same key returns the original document
119+
(and its current status) instead of creating a duplicate. Use this
120+
to make a client/network retry of the upload safe.
121+
parameters:
122+
- name: Idempotency-Key
123+
in: header
124+
required: false
125+
description: |
126+
Opaque client-chosen key, unique per document. Repeating a
127+
request with the same key returns the existing document rather
128+
than ingesting a duplicate.
129+
schema:
130+
type: string
116131
requestBody:
117132
required: true
118133
content:

pkg/db/db.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"time"
1717

1818
"github.com/jackc/pgx/v5"
19+
"github.com/jackc/pgx/v5/pgconn"
1920
"github.com/jackc/pgx/v5/pgxpool"
2021
)
2122

@@ -25,6 +26,10 @@ var migrationsFS embed.FS
2526
// ErrNotFound signals a missing row.
2627
var ErrNotFound = errors.New("db: not found")
2728

29+
// ErrConflict signals a unique-constraint violation (SQLSTATE 23505), e.g.
30+
// a second insert with the same (org_id, idempotency_key).
31+
var ErrConflict = errors.New("db: conflict")
32+
2833
// Pool wraps *pgxpool.Pool with engine-specific helpers.
2934
type Pool struct {
3035
*pgxpool.Pool
@@ -136,5 +141,9 @@ func mapErr(err error) error {
136141
if errors.Is(err, pgx.ErrNoRows) {
137142
return ErrNotFound
138143
}
144+
var pgErr *pgconn.PgError
145+
if errors.As(err, &pgErr) && pgErr.Code == "23505" {
146+
return fmt.Errorf("%w: %s", ErrConflict, pgErr.ConstraintName)
147+
}
139148
return err
140149
}

pkg/db/documents.go

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ type Document struct {
4141
ErrorMessage string
4242
ByteSize int64
4343
Metadata map[string]string
44+
45+
// IdempotencyKey is the optional client-supplied Idempotency-Key.
46+
// When non-empty it is unique per (org_id, idempotency_key): a repeat
47+
// ingest with the same key returns the existing document instead of
48+
// creating a duplicate. Empty means "no dedup" (column stored as NULL).
49+
IdempotencyKey string
4450
CreatedAt time.Time
4551
UpdatedAt time.Time
4652

@@ -76,14 +82,49 @@ func (p *Pool) NewDocument(ctx context.Context, d Document) error {
7682
if storeID == "" {
7783
storeID = NilScope
7884
}
85+
// NULL (not "") when no key, so the partial unique index ignores the row.
86+
var idemKey any
87+
if d.IdempotencyKey != "" {
88+
idemKey = d.IdempotencyKey
89+
}
7990
_, err = p.Exec(ctx, `
80-
INSERT INTO documents (id, org_id, store_id, title, content_type, source_ref, status, byte_size, metadata)
81-
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
82-
string(d.ID), d.OrgID, storeID, d.Title, d.ContentType, d.SourceRef, string(d.Status), d.ByteSize, meta,
91+
INSERT INTO documents (id, org_id, store_id, title, content_type, source_ref, status, byte_size, metadata, idempotency_key)
92+
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)`,
93+
string(d.ID), d.OrgID, storeID, d.Title, d.ContentType, d.SourceRef, string(d.Status), d.ByteSize, meta, idemKey,
8394
)
8495
return mapErr(err)
8596
}
8697

98+
// GetDocumentByIdempotencyKey returns the document an org previously ingested
99+
// under key, or ErrNotFound if none exists. Used to short-circuit a repeat
100+
// ingest (client retry / SDK transport retry) back to the original document
101+
// instead of creating a duplicate.
102+
func (p *Pool) GetDocumentByIdempotencyKey(ctx context.Context, orgID, key string) (*Document, error) {
103+
if orgID == "" {
104+
return nil, fmt.Errorf("GetDocumentByIdempotencyKey: orgID is required")
105+
}
106+
if key == "" {
107+
return nil, ErrNotFound
108+
}
109+
row := p.QueryRow(ctx, `
110+
SELECT id, org_id, store_id, title, content_type, source_ref, status, error_message,
111+
byte_size, metadata, created_at, updated_at, toc_tree
112+
FROM documents WHERE org_id = $1 AND idempotency_key = $2`, orgID, key)
113+
114+
var d Document
115+
var status string
116+
var rawMeta, rawTOC []byte
117+
if err := row.Scan(&d.ID, &d.OrgID, &d.StoreID, &d.Title, &d.ContentType, &d.SourceRef, &status,
118+
&d.ErrorMessage, &d.ByteSize, &rawMeta, &d.CreatedAt, &d.UpdatedAt, &rawTOC); err != nil {
119+
return nil, mapErr(err)
120+
}
121+
d.Status = DocumentStatus(status)
122+
d.Metadata = unmarshalMeta(rawMeta)
123+
d.TOCTree = rawTOC
124+
d.IdempotencyKey = key
125+
return &d, nil
126+
}
127+
87128
// GetDocument fetches a document scoped to an org and (optionally) a
88129
// store. storeID == "" means "don't filter by store" — used by
89130
// header-less / pre-stores callers. A non-empty storeID restricts the

0 commit comments

Comments
 (0)