Rag: Refactor frontend and indexing when doc is uploaded - #872
Merged
Conversation
…acts invalidate_document_artifacts() committed on its own and deleted artifact directories as it went, so it could not be made part of a larger unit of work. update_extractor() needs exactly that: reassigning a document extractor and re-extracting its text have to succeed or fail together. Add commit=False to hand the transaction back to the caller, and defer_paths to collect on-disk artifact paths instead of removing them immediately -- rmtree cannot be rolled back, so the caller deletes them only once its commit has succeeded. Both default to today's behaviour, leaving the three existing call sites unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents were a global, hash-deduplicated library: one `document` row per
distinct file for the whole installation, with `UNIQUE(file_hash)` and no
session column. Which session used a document lived in the JSON list
`GenerativeSession.parameters["documents"]`, so two sessions could share a row
-- and with it one extractor choice and one set of chunks. Changing the
extractor in one session silently re-indexed the other.
Add `session_id` to `document` and swap the unique constraint for
`UNIQUE(session_id, file_hash)`. The same file uploaded into two sessions is
now two rows, each free to pick its own extractor. Uploads happen through
`POST /document/session/{id}`; `parameters["documents"]` stays, mirrored from
the foreign key by the document endpoints, because the chunk-set signature and
the parameter history both read it. Clients can no longer set it: creation
starts a session empty and both endpoints refuse the key rather than silently
dropping it.
Because a session now starts with no documents, `_validate_documents` no
longer demands a non-empty list, and two things fill the gap that rule was
covering: `IndexStatusService` gains a `no_documents` state instead of
promising an indexing run that cannot happen, and the process endpoint refuses
a chat turn on an empty session instead of failing deep inside the retriever
while fitting an index over no text.
Committing an extractor choice becomes correct in the process. It now:
- invalidates unconditionally. The `force` flag was meant to make the user
confirm a destructive re-index, but it asked the dead link table which
sessions were affected, so the confirmation was unreachable and the
invalidation it guarded never ran. A document has one session now, so there
is nobody to warn, and the flag is gone.
- extracts before mutating anything. The new `extractor_id` used to be
committed first, so a failing extractor left the document pointing at one
that had never produced its text, still serving the old chunks.
- reuses `rag_extractor` rows instead of inserting one per save and leaving
every previous one undeletable.
- no longer disagrees with itself about the cache signature. `extract_text`
built it from empty params while instantiating the extractor with the stored
ones, so for any non-default configuration the signature never matched: each
preview missed the cache and destroyed the index.
Deleting a document (or a session) now clears its chunks, retrievers and
embedding matrices, which nothing did before -- `DocumentService.delete` left
directories behind that no code would ever reclaim.
Two things this replaces rather than keeps:
- `rag_document_pipeline_session_link` is dropped. Nothing in production ever
wrote to it, so every reader saw an empty table: `related_sessions` was
always null, `GET /document/related-sessions/{id}` always returned `[]`, and
the upload conflict never named an affected session.
- Files are stored content-addressed under `documents/blobs/<file_hash>`
instead of by their original name. Two different files called `report.pdf`
hashed differently, so both got a row -- but the same path, and the second
upload overwrote the first one's bytes. Per-session copies would have made
that collision routine. Deletion is reference-counted, so sessions holding
identical files share one file and lose it only with the last row.
The migration backfills ownership from each RAG session's parameters: a
document in one session keeps its row, a document in several is cloned per
session (sharing the blob, so no file I/O), and one in none is deleted, since
without the global documents page it would be unreachable forever. It performs
no filesystem changes and logs the paths it abandons.
`CleanupService._other_sessions_with_same_config` goes too, but note what it
did and did not protect. It compared `documents`, which per-session ownership
makes unique, so it could only ever return `False` -- and it was the wrong
question anyway, since it also blocked cleanup for two sessions that merely
shared a configuration. What it left unguarded is the real hazard:
`rag_chunking_model` and `rag_embedding_model` are keyed by
`(class_name, parameters)` alone, so every session that settled on the same
components shares one row, which a new session taking the backend defaults
makes the ordinary case. Deleting those rows is guarded separately, by what
actually references them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A session's prompt is already self-contained: the pipeline builds it from
`parameters["prompt"]`, and the `rag_prompt` row is bookkeeping written after
the fact. Two endpoints undermined that.
`PATCH /prompt/{id}` edited a row in place, but rows are deduplicated by a
hash of their parameters, so one row is shared by every session that landed on
the same template -- editing it rewrote their prompt too. It could also
collide with the hash unique constraint and surface as a 500, and nothing
reads `rag_prompt` at inference time anyway.
`POST /prompt/{id}/sessions/{sid}` cloned a prompt for a session, from the
days of a shared prompt library. It was already broken: it set
`parameters["prompt_id"]` but never `parameters["prompt"]`, and the job filters
`prompt_id` out, so the session kept running its old template. It also planted
a `cloned_for_session` key in the stored parameters purely to dodge the hash
constraint.
`POST /prompt/` and `GET /prompt/` stay: `prompt_id` is still a convenient way
to pick a registered template, and it is resolved into a component ref the
session owns outright.
Retarget the tests accordingly: what matters now is that a session's prompt is
edited through its own parameters, and that two sessions starting from the same
template stay independent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The RAG entry point listed three things to do: start a session, browse documents, browse prompts. The last two were libraries shared across every session, which is no longer what documents are, so both pages are gone and the session's left panel gains everything the documents page could do -- read the file, read its extracted text, choose the extractor. Reading a document happens in a modal rather than a panel. The left panel is too narrow to read extracted text in, and the centre column stays with the conversation: adjusting a session should never take the chat off screen. `DocumentExtractorModal` already had the split view and the schema-driven extractor form, so it becomes `DocumentInspectorModal`, extracts on open instead of waiting to be asked, and loses the confirmation dialog that the backend no longer needs. Fix where "Generative Hub" sits, which is the other half of the same problem. It lives in `SessionBar`'s 64px header, and RAG put that bar in the lower 40% of a split left panel inside an `overflow: auto` box -- so the way out of a session rendered half-way down the column and scrolled out of sight as the lists grew. Extract the header into `GenerativeHubHeader`, mount it above the split, and let both halves shrink instead of fixing their basis. The button also lied: RAG overrode its action to create a session, so a control reading "Generative Hub" did not go to the hub. Those are now two controls, the labelled one going where it says. Move the breadcrumb out of `GenerativeChat` and into the page. The chat is shared with every generative task, so it carried a `taskName === "RAGTask"` check and rendered the trail inside its own centred column -- lower than the same trail on every other RAG tab, which is why the session view looked different. The page renders it at the same inset as the others, and the chat no longer knows RAG exists. Session creation asks for a name and a model. Documents are uploaded into the session afterwards, so the picker is gone; so is the trailing notice listing the backend's default chunker, retriever and prompt, which was a read-only preview of settings the session view shows anyway. `/rag/documents` and `/rag/prompts` redirect to the RAG home for a release rather than 404-ing quietly: there is no catch-all route, so a bookmark would otherwise render a blank page. Also deletes what these changes orphan, including `DocumentDetailPanel` (422 lines that never had an importer) and `DuplicateDocumentDialog`, whose only purpose was to offer the force-overwrite the backend dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four configuration sections were stacked collapsibles, so reaching the
model meant scrolling past chunking and retrieval, and the panel's own chrome
competed with them for height. They become tabs, using the house `PillTabs`
and cards in `ComponentSelector`'s idiom for the chunking and retrieval
presets.
`PresetCardList` borrows that card treatment rather than the component: what
`ComponentSelector` brings besides the card -- search, category chips, download
controls, a grid keyed to viewport breakpoints -- does not apply to a recipe,
and two columns are unreadable in a panel that can be 15% of the window. The
tabs are scrollable for the same reason: the labels are backend-supplied, so a
full-width row would wrap.
Tabbing hides pending edits, so the panel now says where they are: a dot on
each edited tab, a line naming them above Save, and a Discard button, which the
panel never had. Save still sends the whole draft, because the endpoint
replaces every parameter at once. The context budget moves next to Save, since
every section feeds into it and it belonged to no single tab.
Every section stays mounted, hidden rather than unrendered. `GeneratorPicker`
reports whether its model can actually run through a callback, so a tab the user
never opened would have left Save enabled for a model that cannot answer.
The prompt becomes an editor over the session's own template instead of a
picker over a shared library. That library was not safe to edit: prompt rows
are deduplicated by a hash of their parameters, so two sessions that chose the
same template shared one row, and editing it rewrote the other session's
prompt. The registry's built-in templates remain, but only to seed the
template, and seeding is now an explicit choice -- the language select used to
overwrite whatever the user had written as a side effect of changing language.
Placeholder validation moves with it: a template missing `{chunks}` or
`{input}` marks its tab and blocks Save, rather than failing at generation
time. `HighlightedTextarea` and `PlaceholdersList` are reused unchanged, with
an expand button for the times eight rows in a narrow panel are not enough.
This removes the last consumers of `/v1/prompt/`'s list and create endpoints
from the UI, along with `PromptParamsCard`, `NewPromptModal` and the 500-odd
lines of two-way syncing that kept a picker and a session parameter in step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There were no tests under `components/generative` at all, and the two pieces this refactor rewrote most are exactly the ones whose behaviour is easy to break silently. RAGConfigPanel.test.jsx pins the parts that tabbing put at risk: that an edit made on one tab is still reported and still saved from another, that one Save carries every section (the endpoint replaces all parameters at once, so a per-tab save would be a lie), that Discard restores the saved configuration, and that every section stays mounted -- the generator reports whether its model can run through a callback, so a tab nobody opened would leave Save enabled for a model that cannot answer. PromptEditor.test.jsx pins placeholder validation both ways, that edits are written back as a self-contained component ref rather than a reference to a shared row, and that changing the language leaves the template alone -- the old picker reseeded it as a side effect, discarding whatever the user had written. Both use `renderWithProviders`, which supplies the real theme: `PillTabs` reads `theme.palette.ui.box`, which a bare `createTheme()` does not have. The notistack mock has to return one stable object. The panel's loader depends on `enqueueSnackbar`, so a fresh reference per render restarts the load every render and the panel never leaves its spinner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The RAG docs still described the module as it stood two refactors ago: a global documents page, a shared prompt library, a `force` flag on the extractor endpoint, and routes that no longer exist. Bring the documents these changes falsify up to date. 06-document-processing.md gains an Ownership section and rewrites storage, upload, extraction and invalidation: a document belongs to one session, files are content-addressed with reference-counted deletion, and committing an extractor is one transaction with the extraction first. It also records the signature bug that made every preview un-index the session, so the fix is not mistaken for a refactor. 03-frontend-architecture.md is rewritten. It listed the removed pages and the setup form that PR #857 already replaced, and said nothing about why the layout is shaped the way it is -- why the hub header sits above the left-panel split, why the chat is the only centre content, why the config tabs stay mounted. 05-known-limitations.md drops what is now fixed and records what the isolation cost: chunk sets are no longer shared between sessions, and SQLite foreign keys are not enforced at all, so cascades have to come from the ORM. 04-execution-flow.md gains the step where documents are added, and finishes the sentence it has been truncated mid-way through since it was written. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Picking RAG in the generative hub landed on a menu whose only card was "New RAG session", so starting one took two clicks to do a single thing. The menu was a leftover: it used to offer documents and prompts alongside, and both of those now live inside a session. `/app/generative/rag` renders `RAGCreatePage` directly, so arriving from the hub puts you in the form. Existing sessions stay one click away in the left panel, which is where they already were. `/rag/new` redirects to the root, joining the documents and prompts paths, so bookmarks and any missed link still land somewhere sensible. "Back" on the create page now means leaving RAG, since this page *is* the RAG root -- it used to navigate to the menu, which would be itself. `RAGHomePage` is deleted with its `rag.home.*` strings. The one that survives is the "new session" label, which the session view uses as the tooltip on its `+` button; it moves to `rag.create.newSession` rather than keeping a `home` group named after a page that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The routes table still listed `RAGHomePage` at the RAG root and `RAGCreatePage` behind `/rag/new`, which is the two-click flow that just went away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Introduced new services for indexing: IndexStatusService and IndexJobService. - Updated SetupService to handle both indexing and pipeline assembly. - Enhanced error handling and user feedback in the frontend localization for message sending and indexing. - Refactored backend architecture documentation to reflect changes in indexing workflow. - Added tests for RAG indexing, ensuring correct job handling and session safety. - Implemented deferred filesystem operations to ensure file removals follow transaction commits. - Added cross-session safety tests to prevent shared configuration rows from being deleted prematurely. - Improved rollback mechanisms for document uploads and session management.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of concrete correctness/maintainability issues in the new indexing flow implementation that should be fixed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors DashAI’s RAG document lifecycle to be session-scoped (documents belong to exactly one GenerativeSession) and introduces eager, cancelable indexing via a dedicated indexing job, with corresponding backend API, DB model/migration, frontend flow updates, and test/documentation alignment.
Changes:
- Make documents session-owned (
Document.session_id) with per-session deduplication and remove global document/session-link concepts. - Add eager indexing (
POST /api/v1/rag/sessions/{id}/index) tracked viaGenerativeSession.index_job_id, plus index-status reporting that includes live/last job state. - Update the RAG frontend entry + session page to start indexing on document/config changes and poll only while indexing; add/adjust tests and docs accordingly.
File summaries
| File | Description |
|---|---|
| tests/back/RAG/test_RAG_session_configuration_api.py | Updates session creation flow to attach documents after session creation. |
| tests/back/RAG/test_RAG_prompts.py | Removes prompt tests that depended on prompt PATCH/clone endpoints and global documents. |
| tests/back/RAG/test_RAG_prompt_updates.py | Reframes prompt editing to happen through session parameter updates (no shared prompt mutation). |
| tests/back/RAG/test_RAG_pipeline_api_configs.py | Removes global document fixture usage from pipeline config tests. |
| tests/back/RAG/test_document_extractor_api.py | Adapts document upload/dedup and extractor update tests to session-scoped documents. |
| tests/back/RAG/test_deferred_fs.py | Adds unit coverage for deferred filesystem deletions after DB commit. |
| tests/back/RAG/test_cross_encoder_retriever.py | Removes tests for config-matching across sessions that no longer apply. |
| tests/back/RAG/conftest.py | Adds helpers for creating sessions first, then attaching documents, mirroring new ownership rules. |
| tests/back/api/test_session_api.py | Adjusts session parameter update assertions to reflect documents not being set via PUT. |
| docs/RAG/05-known-limitations.md | Documents new constraints (no chunk sharing across sessions) and indexing/validation caveats. |
| docs/RAG/04-execution-flow.md | Updates execution flow to: create session → upload docs → index job → chat. |
| docs/RAG/02-backend-architecture.md | Updates service/job architecture docs for eager indexing + session-scoped documents. |
| docs/RAG/01-overview.md | Updates frontend entry-point paths for the refactored RAG UI. |
| DashAI/front/src/utils/ragValidation.js | Removes old frontend-side deep model-config validation utility. |
| DashAI/front/src/types/ragPrompt.ts | Removes prompt API typing that no longer reflects the flow. |
| DashAI/front/src/types/ragConfiguration.ts | Extends index-status types to include no_documents/indexing + job state payload. |
| DashAI/front/src/pages/generative/RAGSession/RAGSessionPage.test.jsx | Adds tests for eager indexing triggers + polling behavior. |
| DashAI/front/src/pages/generative/RAGSession/RAGSessionPage.jsx | Starts indexing on doc/config changes and polls only while indexing; UI chrome adjustments. |
| DashAI/front/src/pages/generative/RAGSession/components/RAGSectionColumn.jsx | Removes an unused layout wrapper. |
| DashAI/front/src/pages/generative/RAGSession/advanced/NewPromptModal.jsx | Removes prompt creation UI that depended on removed prompt flows. |
| DashAI/front/src/pages/generative/RAG/RAGPromptsPage.jsx | Removes standalone prompts page (prompts now session-parameter-owned). |
| DashAI/front/src/pages/generative/RAG/RAGHomePage.jsx | Removes extra “menu” entry page; creation is now the entry point. |
| DashAI/front/src/pages/generative/RAG/RAGDocumentsPage.jsx | Removes standalone documents page (documents now session-scoped). |
| DashAI/front/src/pages/generative/RAG/RAGCreatePage.test.jsx | Adds coverage for new minimal session creation payload (no documents). |
| DashAI/front/src/pages/generative/RAG/RAGCreatePage.jsx | Refactors RAG entry to create session from name + model only. |
| DashAI/front/src/pages/generative/GenerativeContent.jsx | Refreshes session list on module entry to include standalone-entry changes. |
| DashAI/front/src/components/generative/SessionBox.jsx | Removes old session row component (SessionBar refactor). |
| DashAI/front/src/components/generative/SessionBar.jsx | Extracts header, improves section visibility defaults, supports hiding header. |
| DashAI/front/src/components/generative/RAG/RAGDocumentsPanel.jsx | Removes wrapper in favor of direct DocumentsBar usage. |
| DashAI/front/src/components/generative/RAG/RAGBreadcrumbs.jsx | Removes breadcrumbs for deleted pages and makes RAG root the creation end-node. |
| DashAI/front/src/components/generative/RAG/PromptViewModal.jsx | Removes prompt viewer tied to removed prompts table UI. |
| DashAI/front/src/components/generative/RAG/PromptSelectionTable.jsx | Removes prompt selection table tied to removed prompt flows/pages. |
| DashAI/front/src/components/generative/RAG/PromptEditor.test.jsx | Adds tests ensuring prompt editor writes only params prompt components actually read. |
| DashAI/front/src/components/generative/RAG/PresetCardList.jsx | Adds reusable vertical preset card list UI. |
| DashAI/front/src/components/generative/RAG/DuplicateDocumentDialog.jsx | Removes cross-session duplicate/force UI (dedup is now per-session). |
| DashAI/front/src/components/generative/RAG/DocumentListItem.jsx | Adds row action slot + “indexing” visual state handling. |
| DashAI/front/src/components/generative/RAG/DocumentList.jsx | Makes document list purely presentational; owner controls preview/inspector/actions. |
| DashAI/front/src/components/generative/RAG/DocumentInspectorModal.jsx | Refactors extractor modal into inspector that loads extraction on open and removes force-confirm flow. |
| DashAI/front/src/components/generative/MainGenerativeBox.jsx | Removes unused wrapper component. |
| DashAI/front/src/components/generative/GenerativeHubHeader.jsx | New shared header for consistent “back to hub” affordance. |
| DashAI/front/src/components/generative/GenerativeChat.jsx | Disables composer during indexing; adds error handling for failed send. |
| DashAI/front/src/components/generative/DocumentReferencesModal.jsx | Updates dialog layout/styling and copy affordances for chunk references. |
| DashAI/front/src/components/custom/ComponentSelector.jsx | Adds showFooter option to hide the “components available” footer strip. |
| DashAI/front/src/App.jsx | Makes /app/generative/rag render creation directly and redirects removed pages. |
| DashAI/back/services/RAG/setup_service.py | Splits indexing (build_index) from pipeline assembly to avoid loading LLM weights during indexing. |
| DashAI/back/services/RAG/session_validation_service.py | Rejects setting documents via session payload/PUT; documents managed by document endpoints. |
| DashAI/back/services/RAG/session_defaults_service.py | Updates defaults documentation now that sessions start empty (no documents). |
| DashAI/back/services/RAG/prompt_service.py | Removes in-place prompt update and session-clone flows to avoid cross-session mutation. |
| DashAI/back/services/RAG/index_status_service.py | Adds no_documents/indexing states and includes resolved job status in payload. |
| DashAI/back/services/RAG/index_job_service.py | New helper utilities to resolve/cancel live indexing jobs via the job queue. |
| DashAI/back/services/RAG/embedding_storage_service.py | Makes embedding matrix writes atomic to avoid truncated .npy on killed jobs. |
| DashAI/back/services/RAG/deferred_fs.py | Adds transaction-aware deferred filesystem deletion utilities. |
| DashAI/back/models/RAG/retrievers/dense/dense_retriever.py | Makes dense retriever embedding writes atomic for the same robustness reason. |
| DashAI/back/job/RAG_index_job.py | New eager indexing job that reports progress and avoids LLM instantiation. |
| DashAI/back/dependencies/database/models.py | Adds GenerativeSession.index_job_id, Document.session_id, per-session uniqueness, and ORM cascades. |
| DashAI/back/api/api_v1/schemas/RAG_prompt.py | Removes update schema now that prompt PATCH is gone. |
| DashAI/back/api/api_v1/schemas/document.py | Adds session_id to document response + typed extractor update request body. |
| DashAI/back/api/api_v1/schemas/init.py | Exports new document schema types. |
| DashAI/back/api/api_v1/endpoints/rag.py | Adds indexing endpoint and augments index-status to consult job queue. |
| DashAI/back/api/api_v1/endpoints/prompts.py | Removes prompt PATCH and session-clone endpoints. |
| DashAI/back/api/api_v1/endpoints/documents.py | Makes upload session-scoped, removes global document list/related-sessions endpoints, cancels live index before destructive ops, and schema-validates extractor update. |
| DashAI/alembic/versions/l5m6n7o8p9q0_add_index_job_id_to_generative_session.py | Migration adding index_job_id column. |
Review details
- Files reviewed: 91/91 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ary job status update in RAG indexing
Felipedino
changed the title
cristian-tamblay
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant changes to how RAG documents are managed and indexed, scoping documents to individual sessions and improving the indexing workflow. Key API endpoints and database migrations have been updated to reflect these changes, ensuring that documents are session-specific and that indexing jobs are properly tracked and canceled when necessary.
Session-scoped documents and API changes:
session_idforeign key, replacing the previous global deduplication model. The unique constraint on documents is now(session_id, file_hash), allowing the same file to exist in multiple sessions independently.Indexing job management:
index_job_idcolumn is added togenerative_sessionto track in-flight indexing jobs, allowing the API to manage and cancel indexing operations more reliably.Extractor update workflow:
Other improvements:
These changes ensure that document management is session-aware, indexing is robust and cancelable, and the API is consistent with the new data model.