Skip to content

Commit f003e14

Browse files
Raderganisback
andauthored
Fix claw-eval create response id (#1257)
Co-authored-by: ganisback <370036720@qq.com>
1 parent 2d3f84a commit f003e14

5 files changed

Lines changed: 103 additions & 3 deletions

File tree

api/handler/evaluation.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,29 @@ func (h *EvaluationHandler) GetEvaluation(ctx *gin.Context) {
114114

115115
}
116116

117+
func (h *EvaluationHandler) GetEvaluationByTaskID(ctx *gin.Context) {
118+
currentUser := httpbase.GetCurrentUser(ctx)
119+
taskID := ctx.Param("task_id")
120+
if taskID == "" {
121+
httpbase.BadRequest(ctx, "task_id is required")
122+
return
123+
}
124+
var req = &types.EvaluationGetReq{}
125+
req.TaskID = taskID
126+
req.Username = currentUser
127+
evaluation, err := h.evaluation.GetEvaluation(ctx.Request.Context(), *req)
128+
if err != nil {
129+
slog.ErrorContext(ctx.Request.Context(), "Failed to get evaluation job by task id", slog.Any("error", err))
130+
if errors.Is(err, errorx.ErrForbidden) {
131+
httpbase.ForbiddenError(ctx, err)
132+
return
133+
}
134+
httpbase.ServerError(ctx, err)
135+
return
136+
}
137+
httpbase.OK(ctx, evaluation)
138+
}
139+
117140
// deleteEvaluation godoc
118141
// @Security ApiKey
119142
// @Summary delete model evaluation

api/handler/evaluation_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,21 @@ func TestEvaluationHandler_GetClawEvaluation(t *testing.T) {
121121
tester.ResponseEq(t, 200, tester.OKText, &types.EvaluationRes{ID: 1, TaskType: types.TaskTypeClawEval})
122122
}
123123

124+
func TestEvaluationHandler_GetClawEvaluationByTaskID(t *testing.T) {
125+
tester := NewEvaluationTester(t).WithHandleFunc(func(h *EvaluationHandler) gin.HandlerFunc {
126+
return h.GetEvaluationByTaskID
127+
})
128+
tester.WithUser()
129+
130+
tester.mocks.evaluation.EXPECT().GetEvaluation(tester.Ctx(), types.EvaluationGetReq{
131+
Username: "u",
132+
TaskID: "task-123",
133+
}).Return(&types.EvaluationRes{ID: 123, TaskId: "task-123", TaskType: types.TaskTypeClawEval}, nil)
134+
tester.WithParam("task_id", "task-123").Execute()
135+
136+
tester.ResponseEq(t, 200, tester.OKText, &types.EvaluationRes{ID: 123, TaskId: "task-123", TaskType: types.TaskTypeClawEval})
137+
}
138+
124139
func TestEvaluationHandler_GetForbidden(t *testing.T) {
125140
tester := NewEvaluationTester(t).WithHandleFunc(func(h *EvaluationHandler) gin.HandlerFunc {
126141
return h.GetEvaluation

api/router/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ func createEvaluationRoutes(apiGroup *gin.RouterGroup, middlewareCollection midd
612612
{
613613
evaluationsGroup.POST("", evaluationHandler.RunEvaluation)
614614
evaluationsGroup.DELETE("/:id", evaluationHandler.DeleteEvaluation)
615+
evaluationsGroup.GET("/task/:task_id", evaluationHandler.GetEvaluationByTaskID)
615616
evaluationsGroup.GET("/:id", evaluationHandler.GetEvaluation)
616617
evaluationsGroup.GET("/:id/logs", evaluationHandler.GetLogs)
617618
}

component/evaluation.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,10 +289,25 @@ func (c *evaluationComponentImpl) DeleteEvaluation(ctx context.Context, req type
289289

290290
// get evaluation result
291291
func (c *evaluationComponentImpl) GetEvaluation(ctx context.Context, req types.EvaluationGetReq) (*types.EvaluationRes, error) {
292-
wf, err := c.workflowStore.FindByID(ctx, req.ID)
292+
var (
293+
wf database.ArgoWorkflow
294+
err error
295+
)
296+
if req.TaskID != "" {
297+
wfObj, findErr := c.workflowStore.FindByTaskID(ctx, req.TaskID)
298+
if findErr != nil {
299+
return nil, fmt.Errorf("fail to get evaluation result, %w", findErr)
300+
}
301+
wf = *wfObj
302+
} else {
303+
wf, err = c.workflowStore.FindByID(ctx, req.ID)
304+
}
293305
if err != nil {
294306
return nil, fmt.Errorf("fail to get evaluation result, %w", err)
295307
}
308+
if !isEvaluationResultTaskType(wf.TaskType) {
309+
return nil, errorx.ErrForbiddenMsg("workflow is not an evaluation job")
310+
}
296311
if wf.Username != req.Username {
297312
canRead, err := c.repoComponent.CheckCurrentUserPermission(ctx, req.Username, wf.Username, membership.RoleRead)
298313
if err != nil {
@@ -361,6 +376,15 @@ func (c *evaluationComponentImpl) GetEvaluation(ctx context.Context, req types.E
361376
return res, nil
362377
}
363378

379+
func isEvaluationResultTaskType(taskType types.TaskType) bool {
380+
switch taskType {
381+
case types.TaskTypeEvaluation, types.TaskTypeClawEval:
382+
return true
383+
default:
384+
return false
385+
}
386+
}
387+
364388
func (c *evaluationComponentImpl) OrgEvaluations(ctx context.Context, req *types.OrgEvaluationsReq) ([]types.ArgoWorkFlowRes, int, error) {
365389
if req.CurrentUser != "" {
366390
canRead, err := c.repoComponent.CheckCurrentUserPermission(ctx, req.CurrentUser, req.Namespace, membership.RoleRead)

component/evaluation_claw_test.go

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,12 @@ func TestEvaluationComponent_CreateClawEvaluation_AutoBuiltinJudgeAPIKey(t *test
146146
r.Model == "glm-5.1" &&
147147
r.JudgeApiKey == "gk-builtin-key" &&
148148
r.JudgeBaseURL == "http://aigateway.test/v1"
149-
})).Return(&types.ArgoWorkFlowRes{ID: 2}, nil)
149+
})).Return(&types.ArgoWorkFlowRes{ID: 4, TaskId: "task-123"}, nil)
150150

151151
resp, err := c.CreateEvaluation(ctx, req)
152152
require.NoError(t, err)
153-
require.Equal(t, int64(2), resp.ID)
153+
require.Equal(t, int64(4), resp.ID)
154+
require.Equal(t, "task-123", resp.TaskId)
154155
}
155156

156157
func TestEvaluationComponent_CreateClawEvaluation_AutoJudgeAPIKeyFallback(t *testing.T) {
@@ -317,6 +318,42 @@ func TestEvaluationComponent_GetClawEvaluation(t *testing.T) {
317318
require.ErrorIs(t, err, errorx.ErrForbidden)
318319
}
319320

321+
func TestEvaluationComponent_GetClawEvaluationByTaskID(t *testing.T) {
322+
ctx := context.Background()
323+
c, _, _, mockWorkflow := newTestEvaluationComponentForClaw(t)
324+
325+
mockWorkflow.EXPECT().FindByTaskID(ctx, "task-1").Return(&database.ArgoWorkflow{
326+
ID: 123,
327+
RepoIds: []string{"glm-5.1"},
328+
TaskType: types.TaskTypeClawEval,
329+
Username: "user1",
330+
TaskName: "claw-job",
331+
TaskId: "task-1",
332+
Status: "Succeeded",
333+
}, nil)
334+
335+
res, err := c.GetEvaluation(ctx, types.EvaluationGetReq{TaskID: "task-1", Username: "user1"})
336+
require.NoError(t, err)
337+
require.Equal(t, int64(123), res.ID)
338+
require.Equal(t, "task-1", res.TaskId)
339+
require.Equal(t, types.TaskTypeClawEval, res.TaskType)
340+
}
341+
342+
func TestEvaluationComponent_GetEvaluationByTaskIDRejectsNonEvaluation(t *testing.T) {
343+
ctx := context.Background()
344+
c, _, _, mockWorkflow := newTestEvaluationComponentForClaw(t)
345+
346+
mockWorkflow.EXPECT().FindByTaskID(ctx, "finetune-task").Return(&database.ArgoWorkflow{
347+
ID: 123,
348+
TaskType: types.TaskTypeFinetune,
349+
Username: "user1",
350+
TaskId: "finetune-task",
351+
}, nil)
352+
353+
_, err := c.GetEvaluation(ctx, types.EvaluationGetReq{TaskID: "finetune-task", Username: "user1"})
354+
require.ErrorIs(t, err, errorx.ErrForbidden)
355+
}
356+
320357
func TestEvaluationComponent_GetClawEvaluationSummary(t *testing.T) {
321358
summaryServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
322359
w.Header().Set("Content-Type", "application/json")

0 commit comments

Comments
 (0)