fix(#256): drop pushDocumentToStore duplicate, align rechunk return shape
The frontend had two endpoints pointing at the same operation:
- features/document/api.ts:pushDocumentToStore → POST /push
- features/chunks/api.ts:pushChunksToStore → POST /chunks/push
Backend now exposes a single /chunks/push route (semantically more
accurate — it's chunks that get pushed, not the doc itself), so the
document-side variant is removed. document/store.ts:pushToStore now
delegates to chunks/api.
rechunkDocument returned {jobId} based on the old async-job model.
The new backend rechunk runs synchronously and returns the chunk
list directly. The DocsLibraryPage caller now reports a count
(via new docs.rechunkDone i18n key) instead of a fake job id.
Adds data-e2e=tab-{mode} on the doc workspace tab strip so the
new Karate scenario can target the chunks tab without text matching.
Extends chunks/api.test.ts with HTTP error-propagation guards on
every call — the original 404 bug would have surfaced in CI.
This commit is contained in:
parent
ef80e22342
commit
b4ad874600
8 changed files with 85 additions and 41 deletions
|
|
@ -128,4 +128,50 @@ describe('chunks API', () => {
|
|||
})
|
||||
expect(result).toEqual(response)
|
||||
})
|
||||
|
||||
// ---- Error propagation: each call lets the apiFetch error bubble up so
|
||||
// the store can map it to a user-visible message. The original 404 bug
|
||||
// (#256) silently swallowed the error — these guard tests would have
|
||||
// caught it.
|
||||
describe('error propagation', () => {
|
||||
it('fetchChunks rejects with apiFetch error (404)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('404: Not Found'))
|
||||
await expect(fetchChunks('d1')).rejects.toThrow('404')
|
||||
})
|
||||
|
||||
it('updateChunk rejects with apiFetch error (404)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('404: Not Found'))
|
||||
await expect(updateChunk('d1', 'c1', { text: 'x' })).rejects.toThrow('404')
|
||||
})
|
||||
|
||||
it('mergeChunks rejects with apiFetch error (409)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('409: Conflict'))
|
||||
await expect(mergeChunks('d1', ['a', 'b'])).rejects.toThrow('409')
|
||||
})
|
||||
|
||||
it('splitChunk rejects with apiFetch error (400)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('400: Bad Request'))
|
||||
await expect(splitChunk('d1', 'c1', 0)).rejects.toThrow('400')
|
||||
})
|
||||
|
||||
it('dropChunk rejects with apiFetch error (404)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('404: Not Found'))
|
||||
await expect(dropChunk('d1', 'c1')).rejects.toThrow('404')
|
||||
})
|
||||
|
||||
it('addChunk rejects with apiFetch error (500)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('500: Internal Server Error'))
|
||||
await expect(addChunk('d1', 'x')).rejects.toThrow('500')
|
||||
})
|
||||
|
||||
it('fetchChunkDiff rejects with apiFetch error (404)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('404: Not Found'))
|
||||
await expect(fetchChunkDiff('d1', 'store')).rejects.toThrow('404')
|
||||
})
|
||||
|
||||
it('pushChunksToStore rejects with apiFetch error (503)', async () => {
|
||||
apiFetch.mockRejectedValue(new Error('503: Service Unavailable'))
|
||||
await expect(pushChunksToStore('d1', 'store')).rejects.toThrow('503')
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -6,7 +6,6 @@ import {
|
|||
deleteDocument,
|
||||
getPreviewUrl,
|
||||
rechunkDocument,
|
||||
pushDocumentToStore,
|
||||
fetchDocumentTree,
|
||||
} from './api'
|
||||
|
||||
|
|
@ -72,25 +71,17 @@ describe('document API', () => {
|
|||
expect(getPreviewUrl('abc', 3, 300)).toBe('/api/documents/abc/preview?page=3&dpi=300')
|
||||
})
|
||||
|
||||
it('rechunkDocument calls POST /api/documents/:id/rechunk', async () => {
|
||||
apiFetch.mockResolvedValue({ jobId: 'job-1' })
|
||||
it('rechunkDocument calls POST /api/documents/:id/rechunk and returns chunks', async () => {
|
||||
const chunks = [{ id: 'c1' }, { id: 'c2' }]
|
||||
apiFetch.mockResolvedValue(chunks)
|
||||
|
||||
const result = await rechunkDocument('42')
|
||||
|
||||
expect(apiFetch).toHaveBeenCalledWith('/api/documents/42/rechunk', { method: 'POST' })
|
||||
expect(result).toEqual({ jobId: 'job-1' })
|
||||
})
|
||||
|
||||
it('pushDocumentToStore calls POST /api/documents/:id/push with store', async () => {
|
||||
apiFetch.mockResolvedValue({ jobId: 'job-2' })
|
||||
|
||||
const result = await pushDocumentToStore('42', 'my-store')
|
||||
|
||||
expect(apiFetch).toHaveBeenCalledWith('/api/documents/42/push', {
|
||||
expect(apiFetch).toHaveBeenCalledWith('/api/documents/42/rechunk', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ store: 'my-store' }),
|
||||
body: JSON.stringify({}),
|
||||
})
|
||||
expect(result).toEqual({ jobId: 'job-2' })
|
||||
expect(result).toEqual(chunks)
|
||||
})
|
||||
|
||||
it('fetchDocumentTree calls GET /api/documents/:id/tree', async () => {
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import type { Document, DocTreeNode } from '../../shared/types'
|
||||
import type { DocChunk, Document, DocTreeNode } from '../../shared/types'
|
||||
import { apiFetch } from '../../shared/api/http'
|
||||
|
||||
export function fetchDocuments(): Promise<Document[]> {
|
||||
|
|
@ -27,14 +27,12 @@ export function getPreviewUrl(id: string, page = 1, dpi = 150): string {
|
|||
return `/api/documents/${id}/preview?page=${page}&dpi=${dpi}`
|
||||
}
|
||||
|
||||
export function rechunkDocument(id: string): Promise<{ jobId: string }> {
|
||||
return apiFetch<{ jobId: string }>(`/api/documents/${id}/rechunk`, { method: 'POST' })
|
||||
}
|
||||
|
||||
export function pushDocumentToStore(id: string, store: string): Promise<{ jobId: string }> {
|
||||
return apiFetch<{ jobId: string }>(`/api/documents/${id}/push`, {
|
||||
/** Rechunk the canonical chunkset. Backend runs synchronously and returns
|
||||
* the new chunks — there is no async job to poll. */
|
||||
export function rechunkDocument(id: string): Promise<DocChunk[]> {
|
||||
return apiFetch<DocChunk[]>(`/api/documents/${id}/rechunk`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ store }),
|
||||
body: JSON.stringify({}),
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,10 +7,14 @@ vi.mock('./api', () => ({
|
|||
uploadDocument: vi.fn(),
|
||||
deleteDocument: vi.fn(),
|
||||
rechunkDocument: vi.fn(),
|
||||
pushDocumentToStore: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../chunks/api', () => ({
|
||||
pushChunksToStore: vi.fn(),
|
||||
}))
|
||||
|
||||
import * as api from './api'
|
||||
import * as chunksApi from '../chunks/api'
|
||||
|
||||
describe('useDocumentStore', () => {
|
||||
beforeEach(() => {
|
||||
|
|
@ -138,12 +142,12 @@ describe('useDocumentStore', () => {
|
|||
expect(store.loading).toBe(false)
|
||||
})
|
||||
|
||||
it('rechunk() returns jobId on success', async () => {
|
||||
api.rechunkDocument.mockResolvedValue({ jobId: 'j1' })
|
||||
it('rechunk() returns chunk count on success', async () => {
|
||||
api.rechunkDocument.mockResolvedValue([{ id: 'c1' }, { id: 'c2' }, { id: 'c3' }])
|
||||
const store = useDocumentStore()
|
||||
const result = await store.rechunk('42')
|
||||
expect(api.rechunkDocument).toHaveBeenCalledWith('42')
|
||||
expect(result).toBe('j1')
|
||||
expect(result).toBe(3)
|
||||
})
|
||||
|
||||
it('rechunk() returns null on error', async () => {
|
||||
|
|
@ -154,16 +158,19 @@ describe('useDocumentStore', () => {
|
|||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it('pushToStore() returns jobId on success', async () => {
|
||||
api.pushDocumentToStore.mockResolvedValue({ jobId: 'j2' })
|
||||
it('pushToStore() delegates to chunks/api.pushChunksToStore and returns jobId', async () => {
|
||||
chunksApi.pushChunksToStore.mockResolvedValue({
|
||||
jobId: 'j2',
|
||||
summary: { embeds: 5, tokens: 50 },
|
||||
})
|
||||
const store = useDocumentStore()
|
||||
const result = await store.pushToStore('42', 'my-store')
|
||||
expect(api.pushDocumentToStore).toHaveBeenCalledWith('42', 'my-store')
|
||||
expect(chunksApi.pushChunksToStore).toHaveBeenCalledWith('42', 'my-store')
|
||||
expect(result).toBe('j2')
|
||||
})
|
||||
|
||||
it('pushToStore() returns null on error', async () => {
|
||||
api.pushDocumentToStore.mockRejectedValue(new Error('fail'))
|
||||
chunksApi.pushChunksToStore.mockRejectedValue(new Error('fail'))
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const store = useDocumentStore()
|
||||
const result = await store.pushToStore('42', 'my-store')
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { defineStore } from 'pinia'
|
|||
import { ref } from 'vue'
|
||||
import type { Document } from '../../shared/types'
|
||||
import { appMaxFileSizeMb } from '../../shared/appConfig'
|
||||
import { pushChunksToStore } from '../chunks/api'
|
||||
import * as api from './api'
|
||||
|
||||
export const useDocumentStore = defineStore('document', () => {
|
||||
|
|
@ -65,10 +66,10 @@ export const useDocumentStore = defineStore('document', () => {
|
|||
selectedId.value = id
|
||||
}
|
||||
|
||||
async function rechunk(id: string): Promise<string | null> {
|
||||
async function rechunk(id: string): Promise<number | null> {
|
||||
try {
|
||||
const res = await api.rechunkDocument(id)
|
||||
return res.jobId
|
||||
const chunks = await api.rechunkDocument(id)
|
||||
return chunks.length
|
||||
} catch (e) {
|
||||
error.value = (e as Error).message || 'Failed to rechunk'
|
||||
console.error('Rechunk failed', e)
|
||||
|
|
@ -78,7 +79,7 @@ export const useDocumentStore = defineStore('document', () => {
|
|||
|
||||
async function pushToStore(id: string, store: string): Promise<string | null> {
|
||||
try {
|
||||
const res = await api.pushDocumentToStore(id, store)
|
||||
const res = await pushChunksToStore(id, store)
|
||||
return res.jobId
|
||||
} catch (e) {
|
||||
error.value = (e as Error).message || 'Failed to push to store'
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@
|
|||
:aria-selected="activeMode === m"
|
||||
:disabled="!modeEnabled(m)"
|
||||
:title="!modeEnabled(m) ? t('workspace.modeDisabled') : undefined"
|
||||
:data-e2e="`tab-${m}`"
|
||||
@click="switchMode(m)"
|
||||
>
|
||||
{{ t(`workspace.tabs.${m}`) }}
|
||||
|
|
|
|||
|
|
@ -369,12 +369,10 @@ function clearSelection(): void {
|
|||
async function bulkRechunk(): Promise<void> {
|
||||
const ids = [...selectedIds.value]
|
||||
clearSelection()
|
||||
const jobs = await Promise.all(ids.map((id) => docStore.rechunk(id)))
|
||||
const dispatched = jobs.filter(Boolean)
|
||||
if (dispatched.length) {
|
||||
// Surface job ids as a brief toast via window.alert for now
|
||||
// E9 (RunsPage) will provide proper job tracking
|
||||
window.alert(t('docs.jobDispatched', { jobId: dispatched.join(', ') }))
|
||||
const counts = await Promise.all(ids.map((id) => docStore.rechunk(id)))
|
||||
const succeeded = counts.filter((n): n is number => n !== null).length
|
||||
if (succeeded) {
|
||||
window.alert(t('docs.rechunkDone', { n: succeeded }))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -76,6 +76,7 @@ const messages: Messages = {
|
|||
'docs.pushSubmit': 'Ingérer',
|
||||
'docs.pushCancel': 'Annuler',
|
||||
'docs.jobDispatched': 'Job lancé ({jobId})',
|
||||
'docs.rechunkDone': 'Re-chunkage terminé ({n} document(s))',
|
||||
|
||||
// Doc import (#214)
|
||||
'docsNew.title': 'Importer des documents',
|
||||
|
|
@ -587,6 +588,7 @@ const messages: Messages = {
|
|||
'docs.pushSubmit': 'Ingest',
|
||||
'docs.pushCancel': 'Cancel',
|
||||
'docs.jobDispatched': 'Job dispatched ({jobId})',
|
||||
'docs.rechunkDone': 'Rechunk done ({n} document(s))',
|
||||
|
||||
// Doc import (#214)
|
||||
'docsNew.title': 'Import documents',
|
||||
|
|
|
|||
Loading…
Reference in a new issue