Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"migrate:reset": "prisma migrate reset --schema=./src/core/infrastructure/db/schema.prisma",
"generate": "prisma generate --schema=./src/core/infrastructure/db/schema.prisma",
"logo": "electron-icon-builder --input=./src/renderer/assets/logo.png --output=./public",
"test": "vitest",
"test": "vitest run",
"test:unit": "vitest run --config vitest.config.ts",
"test:renderer": "vitest run --config vitest.config.renderer.ts",
"test:watch": "vitest watch",
Expand Down
22 changes: 20 additions & 2 deletions src/core/application/workers/ConverterWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,14 @@ export class ConverterWorker extends WorkerBase {
// Step 2: Check task is not cancelled
const task = await tx.task.findUnique({
where: { id: page.task },
select: { status: true, pages: true, completed_count: true, failed_count: true },
select: {
status: true,
pages: true,
completed_count: true,
failed_count: true,
provider: true,
model: true,
},
});

if (!task) {
Expand All @@ -393,6 +400,8 @@ export class ConverterWorker extends WorkerBase {
completed_at: new Date(),
worker_id: null, // Release worker
error: null,
provider: task.provider,
model: task.model,
},
});

Expand Down Expand Up @@ -475,7 +484,14 @@ export class ConverterWorker extends WorkerBase {
// Step 2: Check task is not cancelled
const task = await tx.task.findUnique({
where: { id: page.task },
select: { status: true, pages: true, completed_count: true, failed_count: true },
select: {
status: true,
pages: true,
completed_count: true,
failed_count: true,
provider: true,
model: true,
},
});

if (!task) {
Expand All @@ -494,6 +510,8 @@ export class ConverterWorker extends WorkerBase {
error: errorMessage,
completed_at: new Date(),
worker_id: null, // Release worker
provider: task.provider,
model: task.model,
},
});

Expand Down
14 changes: 13 additions & 1 deletion src/core/application/workers/__tests__/ConverterWorker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,21 +585,27 @@ describe('ConverterWorker', () => {
};

it('should update page status to COMPLETED', async () => {
let pageUpdateData: any;

vi.mocked(prisma.$transaction).mockImplementation(async (callback: any) => {

This comment was marked as outdated.

const tx = {
taskDetail: {
findUnique: vi.fn().mockResolvedValue({
worker_id: worker.getWorkerId(),
status: PageStatus.PROCESSING,
}),
update: vi.fn(),
update: vi.fn().mockImplementation((params: any) => {
pageUpdateData = params.data;
}),
},
task: {
findUnique: vi.fn().mockResolvedValue({
status: TaskStatus.PROCESSING,
pages: 10,
completed_count: 5,
failed_count: 0,
provider: 1,

This comment was marked as outdated.

model: 'gpt-4o',
}),
update: vi.fn().mockResolvedValue({
completed_count: 6,
Expand All @@ -618,6 +624,8 @@ describe('ConverterWorker', () => {
await (worker as any).completePageSuccess(mockPage, mockResult);

expect(prisma.$transaction).toHaveBeenCalled();
expect(pageUpdateData.provider).toBe(1);
expect(pageUpdateData.model).toBe('gpt-4o');
});

it('should skip if page already completed (idempotency)', async () => {
Expand Down Expand Up @@ -823,6 +831,8 @@ describe('ConverterWorker', () => {
pages: 10,
completed_count: 5,
failed_count: 1,
provider: 9,
model: 'claude-3-7-sonnet',
}),
update: vi.fn().mockResolvedValue({ failed_count: 2 }),
},
Expand All @@ -839,6 +849,8 @@ describe('ConverterWorker', () => {

expect(pageUpdateData.status).toBe(PageStatus.FAILED);
expect(pageUpdateData.error).toBe('Test error');
expect(pageUpdateData.provider).toBe(9);
expect(pageUpdateData.model).toBe('claude-3-7-sonnet');
});

it('should set task to FAILED when all pages failed', async () => {
Expand Down
28 changes: 22 additions & 6 deletions src/core/infrastructure/services/CloudService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
CloudCancelTaskResponse,
CloudRetryPageResponse,
CloudApiPagination,
CloudModelTier,
PaymentCheckoutApiResponse,
PaymentCheckoutStatusApiResponse,
PaymentHistoryApiItem,
Expand Down Expand Up @@ -433,15 +434,23 @@ class CloudService {
/**
* Retry an entire task (creates a new task)
*/
public async retryTask(id: string): Promise<{
public async retryTask(id: string, model?: CloudModelTier): Promise<{
success: boolean;
data?: CreateTaskResponse;
error?: string;
}> {
try {
const res = await authManager.fetchWithAuth(`${API_BASE_URL}/api/v1/tasks/${encodeURIComponent(id)}/retry`, {
method: 'POST',
});
const hasModelOverride = typeof model === 'string' && model.length > 0;
const res = await authManager.fetchWithAuth(
`${API_BASE_URL}/api/v1/tasks/${encodeURIComponent(id)}/retry`,
hasModelOverride
? {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ model }),
}
: { method: 'POST' }
);

if (!res.ok) {
const errorBody = await res.json().catch(() => null);
Expand Down Expand Up @@ -469,15 +478,22 @@ class CloudService {
/**
* Retry a single page
*/
public async retryPage(taskId: string, pageNumber: number): Promise<{
public async retryPage(taskId: string, pageNumber: number, model?: CloudModelTier): Promise<{
success: boolean;
data?: CloudRetryPageResponse;
error?: string;
}> {
try {
const hasModelOverride = typeof model === 'string' && model.length > 0;
const res = await authManager.fetchWithAuth(
`${API_BASE_URL}/api/v1/tasks/${encodeURIComponent(taskId)}/pages/${encodeURIComponent(String(pageNumber))}/retry`,
{ method: 'POST' },
hasModelOverride
? {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ model }),
}
: { method: 'POST' },
);

if (!res.ok) {
Expand Down
31 changes: 31 additions & 0 deletions src/core/infrastructure/services/__tests__/CloudService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,37 @@ describe('CloudService', () => {
})
})

it('retryTask/retryPage send model override payload when model is provided', async () => {

This comment was marked as outdated.

This comment was marked as outdated.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] CloudService override payload tests miss boundary case when model is omitted

Only the provided-model branch is tested; omission behavior (no body vs empty body) remains unguarded.

Suggestion: Add tests for retryTask(id) and retryPage(id,page) without model to assert exact request options and prevent accidental API contract drift.

Risk: A subtle request-shape regression could break backward compatibility with server endpoints expecting optional model fields.

Confidence: 0.85

[From SubAgent: testing]

const cloudService = (await import('../CloudService.js')).default

mockAuthManager.fetchWithAuth
.mockResolvedValueOnce(makeJsonResponse(200, { success: true, data: { task_id: 'task-2', events_url: '/events' } }))
.mockResolvedValueOnce(makeJsonResponse(200, { success: true, data: { task_id: 'task-1', page: 3, status: 'queued' } }))

await cloudService.retryTask('task-1', 'pro')
await cloudService.retryPage('task-1', 3, 'ultra')

const retryTaskCall = mockAuthManager.fetchWithAuth.mock.calls[0]
expect(retryTaskCall[0]).toContain('/api/v1/tasks/task-1/retry')
expect(retryTaskCall[1]).toEqual(
expect.objectContaining({
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ model: 'pro' }),
})
)

const retryPageCall = mockAuthManager.fetchWithAuth.mock.calls[1]
expect(retryPageCall[0]).toContain('/api/v1/tasks/task-1/pages/3/retry')
expect(retryPageCall[1]).toEqual(
expect.objectContaining({
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ model: 'ultra' }),
})
)
})

it('cancelTask/retryTask/retryPage/deleteTask return API errors when non-OK', async () => {
const cloudService = (await import('../CloudService.js')).default
mockAuthManager.fetchWithAuth
Expand Down
1 change: 1 addition & 0 deletions src/main/ipc/__tests__/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ vi.mock('../../../shared/ipc/channels.js', () => ({
GET_ALL: 'task:getAll',
GET_BY_ID: 'task:getById',
UPDATE: 'task:update',
RETRY: 'task:retry',
DELETE: 'task:delete',
HAS_RUNNING: 'task:hasRunningTasks',
},
Expand Down
13 changes: 12 additions & 1 deletion src/main/ipc/handlers/__tests__/cloud.handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,21 @@ describe('cloud.handler', () => {

expect(mockCloudService.getTasks).toHaveBeenCalledWith(2, 20)
expect(mockCloudService.getTaskById).toHaveBeenCalledWith('t1')
expect(mockCloudService.retryTask).toHaveBeenCalledWith('t1')
expect(mockCloudService.retryTask).toHaveBeenCalledWith('t1', undefined)
expect(mockCloudService.deleteTask).toHaveBeenCalledWith('t1')
})

it('passes model override for retryTask and retryPage', async () => {

This comment was marked as outdated.

mockCloudService.retryTask.mockResolvedValueOnce({ success: true, data: { task_id: 'new' } })
mockCloudService.retryPage.mockResolvedValueOnce({ success: true, data: { task_id: 't1', page: 1, status: 1 } })

await handlers.get('cloud:retryTask')!({}, { id: 't1', model: 'pro' })
await handlers.get('cloud:retryPage')!({}, { taskId: 't1', pageNumber: 1, model: 'ultra' })

expect(mockCloudService.retryTask).toHaveBeenCalledWith('t1', 'pro')
expect(mockCloudService.retryPage).toHaveBeenCalledWith('t1', 1, 'ultra')
})

describe('cloud:downloadPdf', () => {
it('returns service error when download fails', async () => {
mockCloudService.downloadPdf.mockResolvedValueOnce({ success: false, error: 'bad' })
Expand Down
Loading