Skip to content

[fix][evaluation] keep experiment status Terminated after retrying items post-termination - #670

Open
shemingxin66 wants to merge 1 commit into
mainfrom
fix/expt-terminated-retry-statemachine
Open

shemingxin66 wants to merge 1 commit into
mainfrom
fix/expt-terminated-retry-statemachine

Conversation

@shemingxin66

Copy link
Copy Markdown
Collaborator

背景

实验被终止后,用户手动重试特定 item 时,后端状态机出现两个不符合预期的表现(会话 task-746aac4bd6235c89a69534a7dec0b9a0):

  1. 重试个别 item 时,实验状态机从「终止」变回「进行中」,页面显示的执行中题目数 = 实验并发数 + 1(预期应为 1)。
  2. 重试的 item 完成后,实验被塌成「成功/失败」,只显示成功/失败数,不再展示执行中/待执行/终止——即便实验里仍有终止的 item(预期应仍显示「终止」,并分别声明成功/失败/执行中/待执行/终止各计数)。

根因与修复

bug#1(数量脏值):CompleteExpt 里 stats 的计算与写入(CalculateStats + UpdateByExptID)原本发生在 in-flight item 被标记为 Terminal(terminateIncompleteItemRunLogs / terminateItemTurns)之前,导致 stats 表残留虚高的 processing_cnt(≈终止瞬间的并发数)、少算 terminated_cnt。
修复:删除提前的 UpdateByExptID;在终止收口之后,按 item 主表现值重算 CalculateStats 再落 stats 表。仅在真正终止了行时(statsDirty)才重算,正常完成 / Failed 路径复用首次快照、不多做一次全量扫描。

bug#2(收敛判据):终态推导判据(原 design D5)刻意排除 TerminatedItemCnt,导致仍含终止 item 的实验被误判为 Success/Failed。
修复:收敛判据新增最高优先级分支——只要 TerminatedItemCnt > 0 一律收敛为 Terminated,前端仍分别展示 成功/失败/执行中/待执行/终止 各计数(DTO 已透传五类)。

影响面

  • 仅改动 modules/evaluation/domain/service/expt_manage_execution_impl.go 一个实现文件 + 2 个测试文件,74 增 30 删。
  • 无 IDL 变更。
  • legacy 与 enforce 两条计数路径都经 CompleteExpt,一处修复即同时覆盖。

验证

  • go build ./modules/evaluation/... ✅
  • go vet ✅ / gofmt ✅
  • go test ./modules/evaluation/domain/service/ 全包通过(260s ok)✅
  • 重写了原 D5 单测以反映新语义;新增对 bug#1「重算后写入」不变量的断言。

🤖 Generated with Claude Code

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

…ems 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>
@shemingxin66
shemingxin66 force-pushed the fix/expt-terminated-retry-statemachine branch from 92ec0b3 to a3addf4 Compare September 17, 2026 13:41
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.47059% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ation/domain/service/expt_manage_execution_impl.go 76.47% 2 Missing and 2 partials ⚠️

❌ Your patch check has failed because the patch coverage (76.47%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #670   +/-   ##
=======================================
  Coverage        ?   78.83%           
=======================================
  Files           ?      707           
  Lines           ?    87902           
  Branches        ?        0           
=======================================
  Hits            ?    69298           
  Misses          ?    14611           
  Partials        ?     3993           
Flag Coverage Δ
unittests 78.83% <76.47%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ation/domain/service/expt_manage_execution_impl.go 82.49% <76.47%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41c0896...a3addf4. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants