Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions frontend/taskdeck-web/src/composables/useInboxOrchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,23 @@ export function useInboxOrchestrator(options: {
}
inboxLoadPerf.start()
try {
await captureStore.fetchItems({
// `applied` is the store reporting that THIS call's response was written
// into `items` (#2501). `fetchItems` resolves without writing anything
// when its own request id has been superseded, so resolution alone is not
// evidence the new scope's rows arrived. Clearing the flag on resolution
// alone un-hid the retained OLD-scope rows under the NEW scope's label —
// the exact state this flag exists to prevent. The request-id and
// scope-key checks below stay: they guard against a stale caller, while
// `applied` guards against a dropped response.
const applied = await captureStore.fetchItems({
limit: 200,
...(activeBoardId.value ? { boardId: activeBoardId.value } : {}),
})
const currentScopeKey = JSON.stringify({
boardId: activeBoardId.value,
archived: isArchivedHistory.value,
})
if (requestId === latestInboxLoadRequestId && requestScopeKey === currentScopeKey) {
if (applied && requestId === latestInboxLoadRequestId && requestScopeKey === currentScopeKey) {
isScopeReplacement.value = false
}
} catch {
Expand Down
17 changes: 17 additions & 0 deletions frontend/taskdeck-web/src/locales/en/inbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,19 @@
export default {
eyebrow:
'Inbox · capture surface · {pending} awaiting triage · {total} captured | Inbox · capture surface · {pending} awaiting triage · {total} captured',
// Shown INSTEAD of `eyebrow` during a scope replacement (#2501). The rows
// those counts would be computed from belong to the scope the user just left,
// so the eyebrow carries no count at all rather than a count about somewhere
// else. Deliberately not a plural message: it has nothing to agree with,
// which is the whole point.
//
// It also makes NO claim about the load. `isScopeReplacement` is sticky
// across failure by design — the orchestrator swallows the throw so the
// retained rows stay hidden — so a "loading…" word here would sit above the
// table's own error and Retry, permanently, describing a load that had
// already stopped. The table owns loading, error and retry; this line owns
// only the refusal to publish a count it cannot stand behind.
eyebrowUncounted: 'Inbox · capture surface',
// Rendered as `{lead} <em>{emphasis}</em>` — the space before the emphasis
// comes from the template, so `lead` must not carry a trailing space.
title: {
Expand Down Expand Up @@ -132,6 +145,10 @@ export default {
empty: {
scoped: 'No captures in {scope}. Show all captures to restore the full Inbox.',
},
// Appended to the capture-count line while a SAME-scope list load runs over
// rows that stay visible and usable (#2501). Lowercase because it follows a
// "·" separator inside that line.
refreshing: 'refreshing…',
variantToggle: {
label: 'Capture variant',
},
Expand Down
9 changes: 9 additions & 0 deletions frontend/taskdeck-web/src/locales/es/inbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
export default {
eyebrow:
'Inbox · superficie de captura · {pending} por clasificar · {total} capturada | Inbox · superficie de captura · {pending} por clasificar · {total} capturadas',
// Se muestra EN LUGAR de `eyebrow` mientras se sustituye el ámbito (#2501):
// los recuentos serían del ámbito que el usuario acaba de dejar. Sin plural:
// no hay ningún número con el que concordar. Y sin ninguna palabra sobre la
// carga: la tabla es la dueña del estado de carga, del error y del reintento.
eyebrowUncounted: 'Inbox · superficie de captura',
title: {
lead: '¿Qué tienes en mente,',
emphasis: 'en dos palabras?',
Expand Down Expand Up @@ -99,6 +104,10 @@ export default {
empty: {
scoped: 'No hay capturas en {scope}. Muestra todas las capturas para restaurar el Inbox completo.',
},
// Se añade a la línea del recuento durante una recarga en el MISMO ámbito,
// con las filas todavía visibles y utilizables (#2501). En minúscula: va
// detrás de un separador "·".
refreshing: 'actualizando…',
variantToggle: {
label: 'Variante de captura',
},
Expand Down
9 changes: 9 additions & 0 deletions frontend/taskdeck-web/src/locales/it/inbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
export default {
eyebrow:
'Inbox · superficie di cattura · {pending} da smistare · {total} catturato | Inbox · superficie di cattura · {pending} da smistare · {total} catturati',
// Mostrato AL POSTO di `eyebrow` durante la sostituzione dell'ambito (#2501):
// i conteggi apparterrebbero all'ambito appena lasciato. Nessun plurale: non
// c'è alcun numero con cui concordare. E nessuna parola sul caricamento: la
// tabella possiede stato di caricamento, errore e riprova.
eyebrowUncounted: 'Inbox · superficie di cattura',
title: {
lead: 'Cosa hai in mente,',
emphasis: 'in breve?',
Expand Down Expand Up @@ -97,6 +102,10 @@ export default {
empty: {
scoped: 'Nessuna cattura in {scope}. Mostra tutte le catture per ripristinare l’Inbox completo.',
},
// Aggiunto alla riga del conteggio durante un aggiornamento nello STESSO
// ambito, con le righe ancora visibili e utilizzabili (#2501). Minuscolo:
// segue un separatore "·".
refreshing: 'aggiornamento…',
variantToggle: {
label: 'Variante di cattura',
},
Expand Down
31 changes: 27 additions & 4 deletions frontend/taskdeck-web/src/store/captureStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,31 +188,54 @@ export const useCaptureStore = defineStore('capture', () => {
})
}

async function fetchItems(query?: CaptureListQuery) {
/**
* Load the Inbox list, and REPORT whether this call's response was applied
* (#2501).
*
* A superseded call resolves without writing anything — twice over, once in
* the success path and once in the catch — so resolution alone never meant
* "the rows on screen are now this call's rows". A caller that inferred
* success from resolution therefore acted on a response the store had
* dropped; `useInboxOrchestrator` cleared its scope-replacement flag that
* way, un-hiding the retained OLD-scope rows under the NEW scope's label.
*
* `true` means this response was written into `items`. `false` means it was
* dropped as superseded and the caller's assumptions about `items` are
* unchanged. A failure that is still the latest request throws, as before, so
* failure and supersession stay distinguishable. The value is additive:
* existing `await fetchItems(...)` call sites that ignore it are unaffected.
*/
async function fetchItems(query?: CaptureListQuery): Promise<boolean> {
const requestId = ++latestListLoadRequestId
if (isDemoMode) {
loadingList.value = true
listError.value = null
// The guard is pre-existing and cannot currently fail: nothing awaits
// between the id bump above and this check, so no other call can have
// superseded this one. It is kept, with its `false` arm, so the branch
// stays correct and total if the demo path ever becomes genuinely async.
if (requestId === latestListLoadRequestId) {
items.value = buildDemoCaptureItems()
loadingList.value = false
return true
}
return
return false
}

try {
loadingList.value = true
listError.value = null
const loadedItems = await captureApi.listItems(query)
if (requestId !== latestListLoadRequestId) return
if (requestId !== latestListLoadRequestId) return false
// This is the explicit/user-facing list load. A successful mutation may
// finish while a scope replacement is in flight, but that must not make
// the newer scope response disappear. The request id still gives the
// usual latest-load-wins ordering; background batch polls keep the write
// generation guard in their own reader below.
items.value = loadedItems
return true
} catch (e: unknown) {
if (requestId !== latestListLoadRequestId) return
if (requestId !== latestListLoadRequestId) return false
const message = getErrorDisplay(e, 'Failed to load inbox items').message
listError.value = message
toast.error(message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ describe('useInboxOrchestrator', () => {
],
})
mockCaptureStore.pollBatchTriageCompletion.mockReset().mockReturnValue(vi.fn())
// The store reports whether it APPLIED the response (#2501). The default is
// the ordinary case: the response was the latest and was written.
mockCaptureStore.fetchItems.mockReset().mockResolvedValue(true)
})

describe('batch selection', () => {
Expand Down Expand Up @@ -437,7 +440,10 @@ describe('useInboxOrchestrator', () => {
})

it('marks a route-scope list load as a replacement until it succeeds', async () => {
const pendingLoad = deferred<unknown>()
// `true` is the store reporting an APPLIED response (#2501). This test
// used to resolve `undefined`, which passed under both the old contract
// and the new one and so proved nothing about which was in force.
const pendingLoad = deferred<boolean>()
mockCaptureStore.fetchItems.mockReturnValueOnce(pendingLoad.promise)
const orch = createOrchestrator()

Expand All @@ -446,7 +452,7 @@ describe('useInboxOrchestrator', () => {

expect(orch.isScopeReplacement.value).toBe(true)

pendingLoad.resolve(undefined)
pendingLoad.resolve(true)
await flushAsyncWork()

expect(orch.isScopeReplacement.value).toBe(false)
Expand Down Expand Up @@ -815,19 +821,53 @@ describe('useInboxOrchestrator', () => {
})

it('clears the scope-replacement state only after the latest scoped load succeeds', async () => {
const pendingLoad = deferred<void>()
const pendingLoad = deferred<boolean>()
mockCaptureStore.fetchItems.mockReturnValueOnce(pendingLoad.promise)
const orch = createOrchestrator()

const load = orch.loadInboxForScopeReplacement()
expect(orch.isScopeReplacement.value).toBe(true)

pendingLoad.resolve()
// `true` is the store saying it WROTE this response into `items`.
pendingLoad.resolve(true)
await load

expect(orch.isScopeReplacement.value).toBe(false)
})

/**
* #2501 MEDIUM-1: `fetchItems` returns without writing anything when its
* request id has been superseded, and it does so by resolving, not by
* throwing. Resolution alone therefore does not mean the new scope's rows
* arrived, and treating it that way un-hid the retained OLD-scope rows
* under the NEW scope's chip. Only an applied response clears the flag.
*/
it('keeps the scope-replacement state when the store drops a superseded response', async () => {
const pendingLoad = deferred<boolean>()
mockCaptureStore.fetchItems.mockReturnValueOnce(pendingLoad.promise)
const orch = createOrchestrator()

const load = orch.loadInboxForScopeReplacement()
expect(orch.isScopeReplacement.value).toBe(true)

pendingLoad.resolve(false)
await load

expect(orch.isScopeReplacement.value).toBe(true)
})

it('clears the scope-replacement state on the next applied response', async () => {
mockCaptureStore.fetchItems.mockResolvedValueOnce(false).mockResolvedValueOnce(true)
const orch = createOrchestrator()

await orch.loadInboxForScopeReplacement()
expect(orch.isScopeReplacement.value).toBe(true)

await orch.loadInbox()

expect(orch.isScopeReplacement.value).toBe(false)
})

it('keeps the scope-replacement state after failure so retained rows stay hidden', async () => {
mockCaptureStore.fetchItems.mockRejectedValueOnce(new Error('scope load failed'))
const orch = createOrchestrator()
Expand All @@ -840,7 +880,7 @@ describe('useInboxOrchestrator', () => {
it('lets a successful retry clear a failed scope replacement', async () => {
mockCaptureStore.fetchItems
.mockRejectedValueOnce(new Error('scope load failed'))
.mockResolvedValueOnce(undefined)
.mockResolvedValueOnce(true)
const orch = createOrchestrator()

await orch.loadInboxForScopeReplacement()
Expand Down
43 changes: 39 additions & 4 deletions frontend/taskdeck-web/src/tests/store/captureStore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ describe('captureStore', () => {
},
])

await store.fetchItems({ limit: 100 })
// The return value is the store reporting that it WROTE this response
// (#2501). Callers cannot infer it from resolution alone.
await expect(store.fetchItems({ limit: 100 })).resolves.toBe(true)

expect(store.items).toHaveLength(1)
expect(captureApi.listItems).toHaveBeenCalledWith({ limit: 100 })
Expand All @@ -92,13 +94,17 @@ describe('captureStore', () => {
id: 'all-capture', userId: 'u1', boardId: null, status: 'New', source: 'Typed',
textExcerpt: 'visible after clearing scope', createdAt: new Date().toISOString(), processedAt: null,
}])
await unfilteredLoad
// The latest load applied its response.
await expect(unfilteredLoad).resolves.toBe(true)

resolveScoped([{
id: 'scoped-capture', userId: 'u1', boardId: 'board-7', status: 'New', source: 'Typed',
textExcerpt: 'late scoped result', createdAt: new Date().toISOString(), processedAt: null,
}])
await scopedLoad
// The superseded one RESOLVES, but it wrote nothing, and it says so
// (#2501). Resolution alone used to be indistinguishable from success, so a
// caller could not tell a dropped response from an applied one.
await expect(scopedLoad).resolves.toBe(false)

expect(store.items.map((item) => item.id)).toEqual(['all-capture'])
})
Expand Down Expand Up @@ -133,12 +139,41 @@ describe('captureStore', () => {
id: 'late-scoped', userId: 'u1', boardId: 'board-7', status: 'New', source: 'Typed',
textExcerpt: 'obsolete scoped response', createdAt: new Date().toISOString(), processedAt: null,
}])
await scopedLoad
// Superseded by the (failed) unfiltered load, so it applied nothing (#2501).
await expect(scopedLoad).resolves.toBe(false)
expect(store.loadingList).toBe(false)
expect(store.listError).toBe('Failed to load inbox items')
expect(store.items.map((item) => item.id)).toEqual(['retained-scoped'])
})

it('reports a superseded FAILURE as unapplied rather than surfacing it', async () => {
const store = useCaptureStore()
let rejectScoped!: (reason?: unknown) => void
let resolveUnfiltered!: (value: any[]) => void
const scopedResponse = new Promise<any[]>((_resolve, reject) => { rejectScoped = reject })
const unfilteredResponse = new Promise<any[]>((resolve) => { resolveUnfiltered = resolve })
vi.mocked(captureApi.listItems)
.mockReturnValueOnce(scopedResponse as never)
.mockReturnValueOnce(unfilteredResponse as never)

const scopedLoad = store.fetchItems({ boardId: 'board-7', limit: 200 })
const unfilteredLoad = store.fetchItems({ limit: 200 })

resolveUnfiltered([{
id: 'all-capture', userId: 'u1', boardId: null, status: 'New', source: 'Typed',
textExcerpt: 'the scope the user is actually on', createdAt: new Date().toISOString(), processedAt: null,
}])
await expect(unfilteredLoad).resolves.toBe(true)

rejectScoped(new Error('obsolete scoped failure'))
// A failure belonging to a scope the user has already left is deliberately
// neither thrown nor recorded — but it is not silent either: the call
// reports that it applied nothing (#2501).
await expect(scopedLoad).resolves.toBe(false)
expect(store.listError).toBeNull()
expect(store.items.map((item) => item.id)).toEqual(['all-capture'])
})

it('loads and caches capture details', async () => {
const store = useCaptureStore()
vi.mocked(captureApi.getItem).mockResolvedValue({
Expand Down
Loading
Loading