Skip to content

Commit 92ec0b3

Browse files
shemingxin66claude
shemingxin66
and
claude
committed
[fix][evaluation] keep experiment status Terminated after retrying items post-termination
实验被终止后手动重试特定 item 时的两个状态机 bug： 1. 终止时的 stats 计算与写入（CompleteExpt）发生在 in-flight item 被标 Terminal 之前，导致 stats 表残留虚高的 processing_cnt（≈终止瞬间并发数）、少算 terminated_cnt。重试 1 个 item 后页面显示「执行中 = 并发数+1」。 修复：删除提前的 UpdateByExptID；在终止收口（terminateItemTurns）之后按 item 主表现值重算 CalculateStats 再落 stats 表。仅在真正终止了行时重算 （statsDirty），正常完成/Failed 路径复用首次快照、不多做扫描。 2. 重试 item 完成后收敛判据（原 design D5）忽略 terminated 行，把仍含终止 item 的实验塌成 Success/Failed。 修复：收敛判据新增最高优先级分支——只要 TerminatedItemCnt>0 一律收敛为 Terminated，前端仍分别展示 成功/失败/执行中/待执行/终止 各计数。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1133eda commit 92ec0b3

3 files changed

Lines changed: 74 additions & 30 deletions

File tree

‎backend/modules/evaluation/domain/service/expt_item_terminate_test.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -517,16 +517,16 @@ func TestRecordEvalItemRunLogs_RejectsNonTerminalState(t *testing.T) {
517517
assert.ErrorContains(t, err, "invalid item run state")
518518
}
519519

520-
// TestExptMangerImpl_CompleteExpt_TerminatedNotFailed 终态推导双向覆盖（tasks 6.6，design D5）：
521-
// - 有 terminated 行、无 fail 行 → Success（不能因为用户主动终止就把实验判失败）
522-
// - 有 fail 行 → 仍 Failed（不误伤既有失败判定）
520+
// TestExptMangerImpl_CompleteExpt_TerminatedNotFailed 终态推导覆盖：
521+
// 只要实验仍存在 terminated 行（用户终止后重试个别 item 的收尾场景），
522+
// 实验一律收敛为 Terminated，terminated 优先级最高、盖过 fail/success 推导。
523523
func TestExptMangerImpl_CompleteExpt_TerminatedNotFailed(t *testing.T) {
524524
ctx := context.Background()
525525
session := &entity.Session{UserID: "test_user"}
526526
const exptID, spaceID = int64(123), int64(789)
527527
runID := int64(456)
528528

529-
t.Run("terminated_without_fail_is_success", func(t *testing.T) {
529+
t.Run("terminated_without_fail_is_terminated", func(t *testing.T) {
530530
ctrl := gomock.NewController(t)
531531
defer ctrl.Finish()
532532
mgr := newTestExptManager(ctrl)
@@ -552,11 +552,11 @@ func TestExptMangerImpl_CompleteExpt_TerminatedNotFailed(t *testing.T) {
552552

553553
require.NoError(t, mgr.CompleteExpt(ctx, exptID, &runID, spaceID, session))
554554
require.NotNil(t, captured)
555-
assert.Equal(t, entity.ExptStatus_Success, captured.Status,
556-
"D5: terminated 是用户主动放弃，不参与 Failed 推导")
555+
assert.Equal(t, entity.ExptStatus_Terminated, captured.Status,
556+
"仍有 terminated 行时实验一律收敛为 Terminated")
557557
})
558558

559-
t.Run("fail_still_failed", func(t *testing.T) {
559+
t.Run("terminated_with_fail_is_terminated", func(t *testing.T) {
560560
ctrl := gomock.NewController(t)
561561
defer ctrl.Finish()
562562
mgr := newTestExptManager(ctrl)
@@ -582,7 +582,8 @@ func TestExptMangerImpl_CompleteExpt_TerminatedNotFailed(t *testing.T) {
582582

583583
require.NoError(t, mgr.CompleteExpt(ctx, exptID, &runID, spaceID, session))
584584
require.NotNil(t, captured)
585-
assert.Equal(t, entity.ExptStatus_Failed, captured.Status, "有 fail 行仍必须判 Failed")
585+
assert.Equal(t, entity.ExptStatus_Terminated, captured.Status,
586+
"terminated 优先级最高，即便同时有 fail 行也判 Terminated")
586587
})
587588
}
588589

‎backend/modules/evaluation/domain/service/expt_manage_execution_impl.go‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -613,22 +613,15 @@ func (e *ExptMangerImpl) CompleteExpt(ctx context.Context, exptID int64, exptRun
613613
return err
614614
}
615615

616-
if err := e.statsRepo.UpdateByExptID(ctx, exptID, spaceID, &entity.ExptStats{
617-
SuccessItemCnt: int32(stats.SuccessItemCnt),
618-
PendingItemCnt: int32(stats.PendingItemCnt),
619-
FailItemCnt: int32(stats.FailItemCnt),
620-
ProcessingItemCnt: int32(stats.ProcessingItemCnt),
621-
TerminatedItemCnt: int32(stats.TerminatedItemCnt),
622-
}); err != nil {
623-
return err
624-
}
625-
626616
status := opt.Status
627617
if !entity.IsExptFinished(status) {
628-
// ⚠️ 判据中**不含** stats.TerminatedItemCnt（design D5）：terminated 是用户主动放弃的行，不是跑失败。
629-
// 保留它会让「用户终止了 2 行、其余全绿」的实验渲染成红色 Failed 并触发失败侧 webhook。
630-
// 实验级 Kill 不受影响：它走 opt.Status=Terminated，IsExptFinished 为真、根本不进这个分支。
631-
if stats.FailItemCnt > 0 || stats.ProcessingItemCnt > 0 || stats.PendingItemCnt > 0 {
618+
// 只要实验里仍存在终止的 item，实验就必须收敛为 Terminated，而不是塌成 Success/Failed。
619+
// 场景：实验被终止后手动重试个别 item，重试的 item 跑完再次进入 CompleteExpt，
620+
// 此时其余 item 仍处终止态；若忽略 TerminatedItemCnt 会误判成功/失败，
621+
// 前端也就只显示成功/失败数、不再展示终止/执行中/待执行。terminated 优先级最高。
622+
if stats.TerminatedItemCnt > 0 {
623+
status = entity.ExptStatus_Terminated
624+
} else if stats.FailItemCnt > 0 || stats.ProcessingItemCnt > 0 || stats.PendingItemCnt > 0 {
632625
status = entity.ExptStatus_Failed
633626
} else {
634627
status = entity.ExptStatus_Success
@@ -663,6 +656,9 @@ func (e *ExptMangerImpl) CompleteExpt(ctx context.Context, exptID int64, exptRun
663656
// 先置终态它就一条都查不到（静默不释放）。理由见 terminateIncompleteItemRunLogs。
664657
e.terminateIncompleteItemRunLogs(ctx, got, exptRunID)
665658

659+
// statsDirty 标记本次是否真的把 in-flight 行改写成了终态；只有为真时才在收口后
660+
// 重算并回写 stats 表，避免正常完成/Failed 路径多做一次全量扫描。
661+
statsDirty := false
666662
if !opt.NoCompleteItemTurn {
667663
incompleteTurnIDs, err := e.exptResultService.GetIncompleteTurns(ctx, exptID, spaceID, session)
668664
if err != nil {
@@ -685,6 +681,7 @@ func (e *ExptMangerImpl) CompleteExpt(ctx context.Context, exptID int64, exptRun
685681
}
686682
// 在实验行状态更新完成后，更新 ExptTurnResultFilter
687683
if len(terminatedItemIDSet) > 0 {
684+
statsDirty = true
688685
terminatedItemIDs := maps.ToSlice(terminatedItemIDSet, func(k int64, v bool) int64 {
689686
return k
690687
})
@@ -717,6 +714,27 @@ func (e *ExptMangerImpl) CompleteExpt(ctx context.Context, exptID int64, exptRun
717714
}
718715
}
719716

717+
// stats 回写放到终止收口之后：terminateIncompleteItemRunLogs / terminateItemTurns
718+
// 已把 in-flight 行由 Processing 改写成 Terminated，此处必须按 item 主表现值重算，
719+
// 否则 stats 表会残留虚高的 processing_cnt（≈终止瞬间并发数）、少算 terminated_cnt，
720+
// 前端就显示成「执行中 = 并发数 + 1」。只有真正终止了行（statsDirty）才重算，
721+
// 正常完成 / Failed 路径直接复用首次快照，不多做一次全量扫描。
722+
if statsDirty {
723+
stats, err = e.exptResultService.CalculateStats(ctx, exptID, spaceID, session)
724+
if err != nil {
725+
return err
726+
}
727+
}
728+
if err := e.statsRepo.UpdateByExptID(ctx, exptID, spaceID, &entity.ExptStats{
729+
SuccessItemCnt: int32(stats.SuccessItemCnt),
730+
PendingItemCnt: int32(stats.PendingItemCnt),
731+
FailItemCnt: int32(stats.FailItemCnt),
732+
ProcessingItemCnt: int32(stats.ProcessingItemCnt),
733+
TerminatedItemCnt: int32(stats.TerminatedItemCnt),
734+
}); err != nil {
735+
return err
736+
}
737+
720738
exptDo := &entity.Experiment{
721739
ID: exptID,
722740
SpaceID: spaceID,

‎backend/modules/evaluation/domain/service/expt_manage_execution_impl_test.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,15 +1623,31 @@ func TestExptMangerImpl_CompleteExpt(t *testing.T) {
16231623
}, nil)
16241624

16251625
// Mock stats calculation
1626+
// 走 Terminated 且有真实终止行 → 收口后按新语义重算一次：
1627+
// 第一次（终止前）返回带 processing 的旧快照，第二次（重算）返回终止后的正确计数。
1628+
// 断言写入 stats 表的必须是第二次（重算）的值。
1629+
calcCall := 0
16261630
mgr.exptResultService.(*svcMocks.MockExptResultService).
16271631
EXPECT().
16281632
CalculateStats(ctx, int64(123), int64(789), session).
1629-
Return(&entity.ExptCalculateStats{
1630-
SuccessItemCnt: 3,
1631-
FailItemCnt: 1,
1632-
ProcessingItemCnt: 0,
1633-
TerminatedItemCnt: 0,
1634-
}, nil)
1633+
DoAndReturn(func(_ context.Context, _, _ int64, _ *entity.Session) (*entity.ExptCalculateStats, error) {
1634+
calcCall++
1635+
if calcCall == 1 {
1636+
return &entity.ExptCalculateStats{
1637+
SuccessItemCnt: 3,
1638+
FailItemCnt: 1,
1639+
ProcessingItemCnt: 2,
1640+
TerminatedItemCnt: 0,
1641+
}, nil
1642+
}
1643+
return &entity.ExptCalculateStats{
1644+
SuccessItemCnt: 3,
1645+
FailItemCnt: 1,
1646+
ProcessingItemCnt: 0,
1647+
TerminatedItemCnt: 2,
1648+
}, nil
1649+
}).
1650+
Times(2)
16351651

16361652
// Mock incomplete turns retrieval
16371653
mgr.exptResultService.(*svcMocks.MockExptResultService).
@@ -1682,11 +1698,20 @@ func TestExptMangerImpl_CompleteExpt(t *testing.T) {
16821698
return nil
16831699
})
16841700

1685-
// Mock stats update
1701+
// Mock stats update：断言写入的是重算后（第二次 CalculateStats）的计数，
1702+
// processing 归零、terminated=2，正是 bug#1 的核心不变量。
16861703
mgr.statsRepo.(*repoMocks.MockIExptStatsRepo).
16871704
EXPECT().
16881705
UpdateByExptID(ctx, int64(123), int64(789), gomock.Any()).
1689-
Return(nil)
1706+
DoAndReturn(func(_ context.Context, _, _ int64, s *entity.ExptStats) error {
1707+
if s.ProcessingItemCnt != 0 {
1708+
return fmt.Errorf("expected recalculated ProcessingItemCnt=0, got %d", s.ProcessingItemCnt)
1709+
}
1710+
if s.TerminatedItemCnt != 2 {
1711+
return fmt.Errorf("expected recalculated TerminatedItemCnt=2, got %d", s.TerminatedItemCnt)
1712+
}
1713+
return nil
1714+
})
16901715

16911716
// Mock experiment update
16921717
mgr.exptRepo.(*repoMocks.MockIExperimentRepo).

0 commit comments

Comments
 (0)