fix: Duplicate Document Upload Creates Multiple UI components and Database Entries

[VishalSh-Microsoft]

Purpose

This pull request introduces deduplication of uploaded documents and improves the upload experience in both the backend and frontend. The backend now computes a SHA-256 hash for each uploaded document to prevent duplicate processing, while the frontend prevents users from uploading the same file multiple times in a session and improves file tracking and error handling.

Backend deduplication and upsert logic:

• The backend (KernelMemory.cs) now computes a SHA-256 hash of the uploaded document stream to generate a unique documentId. If a document with the same hash already exists, it returns the existing document metadata instead of re-importing it.
• The document repository (DocumentRepository.cs) now uses an upsert operation (ReplaceOneAsync with IsUpsert = true) to ensure documents are inserted or updated based on documentId, preventing duplicates.

Frontend upload improvements and deduplication:

• The upload dialog (uploadButton.tsx) tracks uploaded files using a unique key (name:size:lastModified) and prevents duplicate uploads within the same session by filtering out files that have already been uploaded. [1] [2]
• Improved file tracking by adding a key property to each file and using it as the React key for rendering, ensuring consistent UI updates and better error handling. [1] [2]
• When the upload dialog is closed, the set of uploaded file keys is cleared, allowing uploads to start fresh in a new session.
• If an upload fails, the corresponding file key is removed from the set, allowing the user to retry the upload.

Other improvements:

• Minor refactoring, such as updating imports and type annotations, to improve code clarity and maintainability. [1] [2]

These changes together ensure that duplicate documents are not processed multiple times and that the user experience for uploading files is more robust and intuitive.

Does this introduce a breaking change?

• Yes
• No

Golden Path Validation

• I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Deployment Validation

• I have validated the deployment process successfully and all services are running as expected with this change.

[Copilot]

🟡 Changes recommended

There are concrete error-path and data-integrity issues in the updated upload error handling and upsert/stream-hashing logic that can cause failures or inconsistent records under common conditions.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses duplicate document uploads by introducing content-based deduplication in the backend (SHA-256 hash–based documentId + repository upsert semantics) and session-level duplicate prevention in the frontend upload dialog to avoid duplicate UI entries and repeated upload attempts.

Changes:

• Backend: compute SHA-256 hash of uploaded content to derive a stable documentId, and short-circuit imports when the document already exists.
• Backend: change document persistence to an upsert approach keyed by DocumentId to avoid duplicate inserts.
• Frontend: track uploaded files with a stable per-file key and use it for deduping drops and for React list rendering.

File summaries

File	Description
App/frontend-app/src/components/uploadButton/uploadButton.tsx	Adds per-session client-side deduping and stable React keys for upload UI entries.
App/backend-api/Microsoft.GS.DPS/Storage/Documents/DocumentRepository.cs	Switches document registration to upsert-by-DocumentId to reduce duplicate records.
App/backend-api/Microsoft.GS.DPS/API/KernelMemory/KernelMemory.cs	Derives documentId from content hash and returns existing metadata when already imported.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The deduplication flow is still vulnerable to concurrency/race conditions (and related operational edge cases) that can allow duplicate processing/records despite the stated goal.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

App/frontend-app/src/components/uploadButton/uploadButton.tsx:145

• Closing the dialog while uploads are in progress clears uploadedFileKeys and the UI list, but the async upload loop will continue in the background. If the user reopens the dialog before completion, the cleared key set allows re-uploading the same file while the prior upload is still running.

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The backend buffering approach for non-seekable streams can copy entire uploads into memory, which is a significant reliability risk for large documents.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

App/backend-api/Microsoft.GS.DPS/API/KernelMemory/KernelMemory.cs:87

• The cast to ICollection<KeyValuePair<...>> and Remove() is non-idiomatic and makes thread-safety less clear. ConcurrentDictionary has a dedicated TryRemove API that expresses intent and avoids the cast.

                ((ICollection<KeyValuePair<string, Lazy<Task<DocumentImportedResult>>>>)_documentImports)
                    .Remove(new KeyValuePair<string, Lazy<Task<DocumentImportedResult>>>(documentId, documentImport));

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

There are remaining correctness/maintainability/performance concerns in the updated backend upload path that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

App/backend-api/Microsoft.GS.DPS/Storage/Documents/DocumentRepository.cs:147

• document.__partitionkey = existingDocument.__partitionkey; has no effect here: the upsert filter is on id, and the update definition only applies __partitionkey via SetOnInsert, so this assignment is never used when existingDocument != null (the update path). Keeping it is misleading and suggests partition key might be updated for existing items (which typically isn’t supported/desired).

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Sylvester Kaczmarek (sylvesterkaczmarek)]

One frontend edge case: name:size:lastModified is not a content identity. Two different files can have all three metadata values equal, so the second is silently filtered out and never reaches the backend's SHA-256 deduplication. Could the client avoid treating that tuple as authoritative, or hash content and let the backend decide duplicates?

[Sylvester Kaczmarek (sylvesterkaczmarek)]

The deduplication lock is only process-local. Two identical uploads routed to different API instances can both pass FindByDocumentIdAsync() and invoke _kmClient.ImportDocumentAsync() concurrently for the same deterministic document ID before the repository upsert. Could this claim be made atomic across instances before starting the Kernel Memory pipeline?

[Copilot]

🟡 Changes recommended

The frontend session deduplication described in the PR is not actually implemented (random UUID keys are used and no filtering/tracking prevents re-uploading the same file in a session).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

App/backend-api/Microsoft.GS.DPS/API/KernelMemory/KernelMemory.cs:90

• Using an explicit cast to ICollection<KeyValuePair<...>> to remove from ConcurrentDictionary is hard to read and easy to change incorrectly. ConcurrentDictionary already provides TryRemove(KeyValuePair<...>), which keeps the important behavior of only removing if the key/value pair still matches (avoids removing a newly-added Lazy for the same key).

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite