Skip to content

feat(eval): add durable fleet scheduling with deterministic VM simulation - #5288

Merged
MicroGery merged 1 commit into
mainfrom
feat/eval-fleet-coordinator
Sep 14, 2026
Merged

feat(eval): add durable fleet scheduling with deterministic VM simulation#5288
MicroGery merged 1 commit into
mainfrom
feat/eval-fleet-coordinator

Conversation

@likun666661

@likun666661 likun666661 commented Sep 14, 2026

Copy link
Copy Markdown
Member

Summary

Eval currently queues benchmark task groups in one process. Add a durable coordinator and transport-independent worker loop for distributing groups across fixed worker slots, with remote VMs mocked in this first implementation.

Claims commit before dispatch; cell results and content-addressed artifacts commit independently. Recovery preserves valid cells, bounds infrastructure retries, reconciles coordinator outages, and prevents late or duplicate reports from replacing selected results. Execution, verification, usage, and cleanup evidence remain separate so missing usage alone does not rerun valid work.

A seeded virtual-time simulator exercises the real coordinator/worker code with dropped requests and acknowledgements, VM crash/partition, coordinator restart, and delayed completions. It checks capacity, ownership, attempt ordering, and immutable committed results on every tick. Existing maka eval run behavior is unchanged.

Refs #5284

Verification

  • Added 31 tests, including seeds 0–15 and byte-for-byte replay of seed 42.
  • Eval TypeScript suite: 113 passed, 1 platform-dependent skip.
  • Harbor Python suite with Python 3.13: 70 passed, 12 environment-dependent skips. The default system Python was too old for existing Harbor type syntax.
  • Seed 42 CLI replay: all 120 cells completed, no exhausted cells.
  • Full repository build, lint, format check, typecheck, and knip for apps/desktop and packages/ui passed.
  • ASF header audit and staged commit checks passed.

Checks ran in the existing checkout, which also contains two unrelated, uncommitted model-catalog edits; those edits are not included in this PR. Live VMs, Docker/inference integration, and full unrelated workspace test suites were not run.

Remaining fleet integration

  • Add real remote transport, worker identity/authentication, polling, and local connectivity detection.
  • Connect the existing execution adapters and verify actual worker environment manifests and verifier resource requirements.

The mock-phase library and simulator are usable now; see packages/eval/FLEET.md. No verification-only recovery, within-group straggler optimization, or VM provisioning is included. Worker buffering currently survives network disconnection, not process death. The local coordinator uses a single-writer directory; stale locks require explicit recovery after confirming the old owner is dead.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex authored the implementation, simulation, tests, documentation, and this PR description under human direction. The commit contains a Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…tion

Refs #5284. Implement task-group claims, bounded retries, reconciliation, artifact persistence, and mock worker fault replay.

Generated-by: Codex
@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 14, 2026
@likun666661
likun666661 marked this pull request as ready for review September 14, 2026 08:48
Astro-Han

This comment was marked as outdated.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the implementation and clear scope notes. I approve 11256a5a15ce5d572b7e8b1b1754b759e329c251 as the mock coordinator/worker foundation. This supersedes the approval hold in my earlier review; the two points below are follow-ups, not blockers for this PR.

  1. Recovery scope: Please keep the #5284 discussion explicit about preserving completed subject artifacts centrally and recovering only the failed stage. This version retries the whole cell after verification/environment failure. Approving this foundation does not settle that as the long-term recovery behavior; avoiding another model run when only grading failed remains valuable.
  2. Existing execution integration: When connecting real workers, reuse the existing SubjectAdapter and Harbor/Pier execution path. Verify that execution, verification, usage, cleanup, and artifact facts are produced accurately by that path, including actual environment/verifier resource admission. The simulation validates scheduling behavior but does not establish this production mapping.

No actionable P0–P3 defects were found within the declared mock-library scope. The 31 focused tests passed, including an independent compiled-suite run, and the reviewed head remains unchanged with CI test/audit passing. AI-assisted review with Codex and Reviewer Sol.

中文版

感谢实现和清晰的范围说明。我批准当前版本作为模拟 Coordinator/Worker 的基础实现,替代上一条 review 中暂缓批准的结论。以下两点作为后续跟进,不阻塞本 PR:

  1. 恢复范围: 继续在 #5284 明确保留已完成的模型产物、只恢复失败阶段的目标。当前版本在评分或环境失败后会整题重试;批准这个基础实现,不代表将它确定为长期恢复方式。仅评分失败时,仍应尽量避免再次调用模型。
  2. 真实执行接入: 接入真实 worker 时复用现有 SubjectAdapter 和 Harbor/Pier 执行路径,验证执行、评分、用量、清理和产物事实的准确映射,以及真实环境和评分资源准入。模拟测试证明的是调度行为,尚未证明这条生产接入链路。

当前声明的模拟库范围内未发现 P0–P3;31 项定向测试通过,最新 head 未变化,CI 通过。

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed exact head 11256a5a15ce5d572b7e8b1b1754b759e329c251. This adds an exported mock fleet coordinator/worker library, checksummed local state and content-addressed artifacts, bounded cell retries, recovery reconciliation, and a deterministic VM/network fault simulation.

Technical conclusion: NO-GO (2×P2). The coordinator state machine preserves partial commits and selection identity, but the worker control loop does not keep liveness independent from data-plane work, and one permanently invalid executor output can poison the outbox forever.

Required conclusion:

  1. The current solution is not yet optimal for the stated durable worker contract because valid completed work can be discarded solely while its artifact/report upload is still in progress.
  2. Production code deletable: none identified.
  3. Low-quality tests deletable: none identified. Add focused regressions for a transport operation longer than the lease and for one malformed output preceding a valid sibling; the current simulator models immediate success/failure, not these paths.
  4. A focused worker-loop refactor is required: heartbeat renewal must be independently progressable from artifact/report flushing, and report semantics must be validated or converted to a terminal local execution failure before entering the retrying outbox. The coordinator state machine does not need replacement.
  5. This revision is not ready to merge until both P2 paths are fixed and covered.
  6. Residual gaps: this remains a mock-only API; no real VM transport, existing SubjectAdapter/Harbor/Pier mapping, native Windows/macOS persistence path, or verification-only recovery was exercised. The recovery-scope choice remains a product decision in #5284.

This diff changes a public exported API and evaluation correctness behavior, so it needs independent human review under CONTRIBUTING.md.

Validation: build:test, full typecheck/lint/format, ASF headers, changed-file Biome, diff check, Eval 113 pass / 1 platform skip plus Python 72 pass / 10 skip, focused fleet 31/31, hosted test/audit, and a clean merge with current main bf6e9422960401da6bba01d03a8ea9979722bb5a.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

for (const work of heartbeat.work ?? []) this.#adopt(work);
for (const [attemptId, output] of this.#outbox) {
const artifacts = [];
for (const bytes of output.artifacts) artifacts.push(await this.transport.putArtifact(bytes));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Keep lease heartbeats independent of artifact/report delivery

poll() memoizes this entire serial cycle, so while one putArtifact or report command is pending, later calls return the same promise and cannot send another heartbeat. With a valid one-cell result, leaseMs=10, and only the artifact upload held until tick 10, the coordinator expired the assignment; after the upload completed, the valid report was recorded as late, and a one-attempt run settled as incomplete/exhausted. The same failure is reachable through one slow upload/report or enough cells serialized here. Keep lease renewal on an independently progressable path (and bound transport operations), then add a delayed-upload regression that proves valid completed work remains selectable.

throw new Error('executor emitted an unknown or duplicate attempt');
}
run.emitted.add(attemptId);
this.#outbox.set(attemptId, structuredClone(output));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Do not let one permanently invalid output poison the worker outbox

emit() marks the attempt emitted and queues it before any runtime validation. If the future execution adapter produces a type-correct but semantically inconsistent report, the coordinator rejects it permanently, yet the item remains first in this map. In an exact-head probe using usage: "missing" with non-null token usage followed by a valid sibling, five polls all failed with missing usage has tokens; both items remained buffered and pending, while each heartbeat renewed the assignment to 105. The group and worker slot therefore never settle or expire. Validate the report before marking it emitted, or classify permanent protocol rejection into a terminal local execution failure/quarantine so later valid reports and retry recovery can progress.

@MicroGery
MicroGery merged commit a753fa6 into main Sep 14, 2026
4 checks passed
@MicroGery
MicroGery deleted the feat/eval-fleet-coordinator branch September 14, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants