Skip to content

Commit c846c57

Browse files
darkskygitdarkskygit
committed
fix(server): list context status (#12771)
<!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of document statuses to ensure documents without a finished or existing status are now explicitly marked as "processing" instead of remaining undefined. - **Tests** - Added comprehensive new tests and snapshot entries to verify document status merging, including edge cases and concurrent operations, ensuring robust and consistent behavior. - **Enhancements** - Updated context document listings to display the processing status for relevant documents. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent e82c9d2 commit c846c57

File tree

6 files changed

+368
-1
lines changed

6 files changed

+368
-1
lines changed

‎packages/backend/server/src/__tests__/__snapshots__/copilot.e2e.ts.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ Generated by [AVA](https://avajs.dev).
5252
[
5353
{
5454
id: 'docId1',
55+
status: 'processing',
5556
},
5657
]
5758

‎packages/backend/server/src/__tests__/__snapshots__/copilot.e2e.ts.snap‎

6 Bytes
Binary file not shown.

‎packages/backend/server/src/__tests__/models/__snapshots__/copilot-context.spec.ts.md‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,195 @@ Generated by [AVA](https://avajs.dev).
5353
> should return true when embedding table is available
5454
5555
true
56+
57+
## should merge doc status correctly
58+
59+
> basic doc status merge
60+
61+
[
62+
{
63+
id: 'doc1',
64+
status: 'processing',
65+
},
66+
{
67+
id: 'doc2',
68+
status: 'processing',
69+
},
70+
{
71+
id: 'doc3',
72+
status: 'failed',
73+
},
74+
{
75+
id: 'doc4',
76+
status: 'processing',
77+
},
78+
]
79+
80+
> mixed doc status merge
81+
82+
[
83+
{
84+
id: 'doc5',
85+
status: 'finished',
86+
},
87+
{
88+
id: 'doc5',
89+
status: 'finished',
90+
},
91+
{
92+
id: 'doc6',
93+
status: 'processing',
94+
},
95+
{
96+
id: 'doc6',
97+
status: 'failed',
98+
},
99+
{
100+
id: 'doc7',
101+
status: 'processing',
102+
},
103+
]
104+
105+
> edge cases results
106+
107+
[
108+
{
109+
case: 0,
110+
length: 1,
111+
statuses: [
112+
'processing',
113+
],
114+
},
115+
{
116+
case: 1,
117+
length: 1,
118+
statuses: [
119+
'processing',
120+
],
121+
},
122+
{
123+
case: 2,
124+
length: 100,
125+
statuses: [
126+
'processing',
127+
'processing',
128+
'processing',
129+
'processing',
130+
'processing',
131+
'processing',
132+
'processing',
133+
'processing',
134+
'processing',
135+
'processing',
136+
'processing',
137+
'processing',
138+
'processing',
139+
'processing',
140+
'processing',
141+
'processing',
142+
'processing',
143+
'processing',
144+
'processing',
145+
'processing',
146+
'processing',
147+
'processing',
148+
'processing',
149+
'processing',
150+
'processing',
151+
'processing',
152+
'processing',
153+
'processing',
154+
'processing',
155+
'processing',
156+
'processing',
157+
'processing',
158+
'processing',
159+
'processing',
160+
'processing',
161+
'processing',
162+
'processing',
163+
'processing',
164+
'processing',
165+
'processing',
166+
'processing',
167+
'processing',
168+
'processing',
169+
'processing',
170+
'processing',
171+
'processing',
172+
'processing',
173+
'processing',
174+
'processing',
175+
'processing',
176+
'processing',
177+
'processing',
178+
'processing',
179+
'processing',
180+
'processing',
181+
'processing',
182+
'processing',
183+
'processing',
184+
'processing',
185+
'processing',
186+
'processing',
187+
'processing',
188+
'processing',
189+
'processing',
190+
'processing',
191+
'processing',
192+
'processing',
193+
'processing',
194+
'processing',
195+
'processing',
196+
'processing',
197+
'processing',
198+
'processing',
199+
'processing',
200+
'processing',
201+
'processing',
202+
'processing',
203+
'processing',
204+
'processing',
205+
'processing',
206+
'processing',
207+
'processing',
208+
'processing',
209+
'processing',
210+
'processing',
211+
'processing',
212+
'processing',
213+
'processing',
214+
'processing',
215+
'processing',
216+
'processing',
217+
'processing',
218+
'processing',
219+
'processing',
220+
'processing',
221+
'processing',
222+
'processing',
223+
'processing',
224+
'processing',
225+
'processing',
226+
],
227+
},
228+
]
229+
230+
## should handle concurrent mergeDocStatus calls
231+
232+
> concurrent calls results
233+
234+
[
235+
{
236+
call: 1,
237+
status: 'finished',
238+
},
239+
{
240+
call: 2,
241+
status: 'finished',
242+
},
243+
{
244+
call: 3,
245+
status: 'processing',
246+
},
247+
]

‎packages/backend/server/src/__tests__/models/__snapshots__/copilot-context.spec.ts.snap‎

1.16 KB
Binary file not shown.

‎packages/backend/server/src/__tests__/models/copilot-context.spec.ts‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import { randomUUID } from 'node:crypto';
22

33
import { AiSession, PrismaClient, User, Workspace } from '@prisma/client';
44
import ava, { TestFn } from 'ava';
5+
import Sinon from 'sinon';
56

67
import { Config } from '../../base';
8+
import { ContextEmbedStatus } from '../../models/common/copilot';
79
import { CopilotContextModel } from '../../models/copilot-context';
810
import { CopilotSessionModel } from '../../models/copilot-session';
911
import { CopilotWorkspaceConfigModel } from '../../models/copilot-workspace';
@@ -236,3 +238,173 @@ test('should check embedding table', async t => {
236238
// t.false(ret, 'should return false when embedding table is not available');
237239
// }
238240
});
241+
242+
test('should merge doc status correctly', async t => {
243+
const createDoc = (id: string, status?: string) => ({
244+
id,
245+
createdAt: Date.now(),
246+
...(status && { status: status as any }),
247+
});
248+
249+
const createDocWithEmbedding = async (docId: string) => {
250+
await t.context.db.snapshot.create({
251+
data: {
252+
workspaceId: workspace.id,
253+
id: docId,
254+
blob: Buffer.from([1, 1]),
255+
state: Buffer.from([1, 1]),
256+
updatedAt: new Date(),
257+
createdAt: new Date(),
258+
},
259+
});
260+
261+
await t.context.copilotContext.insertWorkspaceEmbedding(
262+
workspace.id,
263+
docId,
264+
[
265+
{
266+
index: 0,
267+
content: 'content',
268+
embedding: Array.from({ length: 1024 }, () => 1),
269+
},
270+
]
271+
);
272+
};
273+
274+
const emptyResult = await t.context.copilotContext.mergeDocStatus(
275+
workspace.id,
276+
[]
277+
);
278+
t.deepEqual(emptyResult, []);
279+
280+
const basicDocs = [
281+
createDoc('doc1'),
282+
createDoc('doc2'),
283+
createDoc('doc3', 'failed'),
284+
createDoc('doc4', 'processing'),
285+
];
286+
const basicResult = await t.context.copilotContext.mergeDocStatus(
287+
workspace.id,
288+
basicDocs
289+
);
290+
t.snapshot(
291+
basicResult.map(d => ({ id: d.id, status: d.status })),
292+
'basic doc status merge'
293+
);
294+
295+
{
296+
await createDocWithEmbedding('doc5');
297+
298+
const mixedDocs = [
299+
createDoc('doc5'),
300+
createDoc('doc5', 'processing'),
301+
createDoc('doc6'),
302+
createDoc('doc6', 'failed'),
303+
createDoc('doc7'),
304+
];
305+
const mixedResult = await t.context.copilotContext.mergeDocStatus(
306+
workspace.id,
307+
mixedDocs
308+
);
309+
t.snapshot(
310+
mixedResult.map(d => ({ id: d.id, status: d.status })),
311+
'mixed doc status merge'
312+
);
313+
314+
const hasEmbeddingStub = Sinon.stub(
315+
t.context.copilotContext,
316+
'hasWorkspaceEmbedding'
317+
).resolves(new Set<string>());
318+
319+
const stubResult = await t.context.copilotContext.mergeDocStatus(
320+
workspace.id,
321+
[createDoc('doc5')]
322+
);
323+
t.is(stubResult[0].status, ContextEmbedStatus.processing);
324+
325+
hasEmbeddingStub.restore();
326+
}
327+
328+
{
329+
const testCases = [
330+
{
331+
workspaceId: 'invalid-workspace',
332+
docs: [{ id: 'doc1', createdAt: Date.now() }],
333+
},
334+
{
335+
workspaceId: workspace.id,
336+
docs: [{ id: 'doc1', createdAt: Date.now(), status: undefined as any }],
337+
},
338+
{
339+
workspaceId: workspace.id,
340+
docs: Array.from({ length: 100 }, (_, i) => ({
341+
id: `doc-${i}`,
342+
createdAt: Date.now() + i,
343+
})),
344+
},
345+
];
346+
347+
const results = await Promise.all(
348+
testCases.map(testCase =>
349+
t.context.copilotContext.mergeDocStatus(
350+
testCase.workspaceId,
351+
testCase.docs
352+
)
353+
)
354+
);
355+
356+
t.snapshot(
357+
results.map((result, index) => ({
358+
case: index,
359+
length: result.length,
360+
statuses: result.map(d => d.status),
361+
})),
362+
'edge cases results'
363+
);
364+
}
365+
});
366+
367+
test('should handle concurrent mergeDocStatus calls', async t => {
368+
await t.context.db.snapshot.create({
369+
data: {
370+
workspaceId: workspace.id,
371+
id: 'concurrent-doc',
372+
blob: Buffer.from([1, 1]),
373+
state: Buffer.from([1, 1]),
374+
updatedAt: new Date(),
375+
createdAt: new Date(),
376+
},
377+
});
378+
379+
await t.context.copilotContext.insertWorkspaceEmbedding(
380+
workspace.id,
381+
'concurrent-doc',
382+
[
383+
{
384+
index: 0,
385+
content: 'content',
386+
embedding: Array.from({ length: 1024 }, () => 1),
387+
},
388+
]
389+
);
390+
391+
const concurrentDocs = [
392+
[{ id: 'concurrent-doc', createdAt: Date.now() }],
393+
[{ id: 'concurrent-doc', createdAt: Date.now() + 1000 }],
394+
[{ id: 'non-existent-doc', createdAt: Date.now() }],
395+
];
396+
397+
const results = await Promise.all(
398+
concurrentDocs.map(docs =>
399+
t.context.copilotContext.mergeDocStatus(workspace.id, docs)
400+
)
401+
);
402+
403+
t.snapshot(
404+
results.map((result, index) => ({
405+
call: index + 1,
406+
status: result[0].status,
407+
})),
408+
'concurrent calls results'
409+
);
410+
});

‎packages/backend/server/src/models/copilot-context.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ export class CopilotContextModel extends BaseModel {
9191
const status = finishedDoc.has(doc.id)
9292
? ContextEmbedStatus.finished
9393
: undefined;
94-
doc.status = status || doc.status;
94+
// NOTE: when the document has not been synchronized to the server or is in the embedding queue
95+
// the status will be empty, fallback to processing if no status is provided
96+
doc.status = status || doc.status || ContextEmbedStatus.processing;
9597
}
9698

9799
return docs;

0 commit comments

Comments
 (0)