fix: address code review issues and cleanup QA reports
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
Code review fixes: - Add error logging to LlmProviderAdminController.testProvider catch block - Use atomic increment operations in TokenBudgetService.updateUsage to prevent race conditions - Update test expectations for atomic increment pattern Cleanup: - Remove obsolete QA automation reports All 1169 tests passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -10,6 +10,7 @@ import {
|
||||
HttpStatus,
|
||||
NotFoundException,
|
||||
BadRequestException,
|
||||
Logger,
|
||||
} from "@nestjs/common";
|
||||
import type { InputJsonValue } from "@prisma/client/runtime/library";
|
||||
import { PrismaService } from "../prisma/prisma.service";
|
||||
@@ -43,6 +44,8 @@ import { CreateLlmProviderDto, UpdateLlmProviderDto, LlmProviderResponseDto } fr
|
||||
*/
|
||||
@Controller("llm/admin")
|
||||
export class LlmProviderAdminController {
|
||||
private readonly logger = new Logger(LlmProviderAdminController.name);
|
||||
|
||||
constructor(
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly llmManager: LlmManagerService
|
||||
@@ -245,8 +248,11 @@ export class LlmProviderAdminController {
|
||||
return {
|
||||
healthy: health.healthy,
|
||||
};
|
||||
} catch {
|
||||
} catch (error: unknown) {
|
||||
// Provider not loaded in manager (might be disabled)
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
this.logger.warn(`Failed to test provider ${id}: ${errorMessage}`);
|
||||
|
||||
return {
|
||||
healthy: false,
|
||||
error: "Provider not loaded in manager. Try reloading providers.",
|
||||
|
||||
@@ -112,9 +112,9 @@ describe("TokenBudgetService", () => {
|
||||
expect(mockPrismaService.tokenBudget.update).toHaveBeenCalledWith({
|
||||
where: { taskId: mockTaskId },
|
||||
data: {
|
||||
inputTokensUsed: 60000,
|
||||
outputTokensUsed: 40000,
|
||||
totalTokensUsed: 100000,
|
||||
inputTokensUsed: { increment: 10000 },
|
||||
outputTokensUsed: { increment: 10000 },
|
||||
totalTokensUsed: { increment: 20000 },
|
||||
budgetUtilization: expect.closeTo(0.667, 2),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -37,6 +37,7 @@ export class TokenBudgetService {
|
||||
|
||||
/**
|
||||
* Update usage after agent response
|
||||
* Uses atomic increment operations to prevent race conditions
|
||||
*/
|
||||
async updateUsage(
|
||||
taskId: string,
|
||||
@@ -47,7 +48,7 @@ export class TokenBudgetService {
|
||||
`Updating usage for task ${taskId}: +${String(inputTokens)} input, +${String(outputTokens)} output`
|
||||
);
|
||||
|
||||
// Get current budget
|
||||
// First verify budget exists
|
||||
const budget = await this.prisma.tokenBudget.findUnique({
|
||||
where: { taskId },
|
||||
});
|
||||
@@ -56,21 +57,18 @@ export class TokenBudgetService {
|
||||
throw new NotFoundException(`Token budget not found for task ${taskId}`);
|
||||
}
|
||||
|
||||
// Calculate new totals
|
||||
const newInputTokens = budget.inputTokensUsed + inputTokens;
|
||||
const newOutputTokens = budget.outputTokensUsed + outputTokens;
|
||||
const newTotalTokens = newInputTokens + newOutputTokens;
|
||||
|
||||
// Calculate utilization
|
||||
// Use atomic increment operations to prevent race conditions
|
||||
const totalIncrement = inputTokens + outputTokens;
|
||||
const newTotalTokens = budget.totalTokensUsed + totalIncrement;
|
||||
const utilization = newTotalTokens / budget.allocatedTokens;
|
||||
|
||||
// Update budget
|
||||
// Update budget with atomic increments
|
||||
const updatedBudget = await this.prisma.tokenBudget.update({
|
||||
where: { taskId },
|
||||
data: {
|
||||
inputTokensUsed: newInputTokens,
|
||||
outputTokensUsed: newOutputTokens,
|
||||
totalTokensUsed: newTotalTokens,
|
||||
inputTokensUsed: { increment: inputTokens },
|
||||
outputTokensUsed: { increment: outputTokens },
|
||||
totalTokensUsed: { increment: totalIncrement },
|
||||
budgetUtilization: utilization,
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user