fix(dspy): guard finetune worker set_result against cancelled job

[detail-app]

Warning

GitHub issue creation failed

Detail attempted to publish this bug to GitHub, but the issue could not be created. This fix PR was created without that issue, and missing tracker references are shown as Unknown issue.

You can review and merge this PR normally. Please review your tracker integration settings before the next publish run.

Detail bug report: View on Detail

📝 Changes Description

This MR/PR contains the following changes:

• dspy/clients/lm.py — LM._run_finetune_job now guards both job.set_result(...) calls with if not job.cancelled():, fixing a concurrent.futures.InvalidStateError worker-thread crash when TrainingJob.cancel() is invoked during a fine-tune.
• tests/clients/test_openai_provider.py — new regression tests covering the finetune success path, error path, and the two cancel timing windows.

Bug: LM._run_finetune_job never calls set_running_or_notify_cancel(), so the TrainingJob Future stays PENDING for the entire remote fine-tune. TrainingJobOpenAI.cancel() (a public method on the TrainingJob returned by LM.finetune()) transitions the Future to CANCELLED via super().cancel(). When the worker later reaches job.set_result(lm) (success path) or job.set_result(err) (error path), concurrent.futures raises InvalidStateError. In the error path this second InvalidStateError is raised inside the except clause, so it escapes the worker thread and kills it; the caller's job.result() then surfaces CancelledError. Any cancel() invoked while the remote job is still running deterministically produces this crash.

Fix: Guard the two set_result calls so they are skipped when the job was cancelled. The caller's result() then raises CancelledError cleanly, and no InvalidStateError escapes the worker thread. The success and error paths for non-cancelled jobs are unchanged.

This is the minimal fix recommended in the bug report; it does not address the separate, theoretical "post-cancel remote-job creation" cost concern (Window 2a), which the report explicitly defers pending live-account assessment.

Closes Unknown issue

✅ Contributor Checklist

• Pre-Commit checks are passing (locally and remotely) — uv run ruff check and uv run ruff format --check pass on the changed files; uv run pytest passes.
• Title of your PR / MR corresponds to the required format
• Commit message follows required format fix(dspy): guard finetune worker set_result against cancelled job

⚠️ Warnings

AI-assisted contribution disclaimer: Per CONTRIBUTING.md §"AI-Generated Contributions", this change was prepared by Detail: Automatic Fixes. I understand and have verified every line: the fix is a two-line guard matching the concurrent.futures Future cancellation protocol, and the tests reproduce the crash deterministically via the real LM.finetune → _run_finetune_job → OpenAIProvider.finetune → TrainingJobOpenAI.cancel paths with only the openai.* SDK boundary mocked. The bug was reproduced before fixing and confirmed fixed after.

Testing summary:

• New regression tests (tests/clients/test_openai_provider.py, 4 tests) — Drive the real production worker paths with the openai.* SDK boundary mocked; threading.Event pairs deterministically pin the worker at each timing window. All 4 pass.
  • Success path (no cancel) returns the trained dspy.LM via result().
  • Error path (provider raises, no cancel) surfaces the exception via result().
  • Cancel during polling (provider_job_id set) — no uncaught exception, remote cancel API invoked, result() raises CancelledError.
  • Cancel during upload_data (provider_file_id not yet set) — no uncaught exception, result() raises CancelledError.
• Bug-catching verification — Reverting the two guards makes the two cancel-window tests fail with InvalidStateError('CANCELLED: <TrainingJobOpenAI ... state=cancelled>') escaping the worker thread (the exact defect), confirming the tests are genuine regression guards.
• Routine checks — ruff check and ruff format --check pass on the changed files; existing tests/clients (159 passed, 22 credential-gated skips, pre-existing) and tests/teleprompt/test_bootstrap_finetune.py (5 passed) show no regressions; full default-gated CI-equivalent suite (pytest -m 'not extra and not deno') passed (1255 passed, 252 skipped, 2 xfailed).
• Ctrl-C forward-compatibility — The standing TODO(enhance): We should listen for keyboard interrupts at dspy/clients/lm.py:382-383 plans to wire Ctrl-C to TrainingJob.cancel(). A standalone script exercising the real worker (mocked SDK boundary) invoked cancel() during polling and confirmed the worker exits cleanly with cancelled() True, no uncaught exception, and result() raises CancelledError — not versioned, as it duplicates the polling-window regression test.

Procedure not verified:

• Live OpenAI end-to-end cancel — requires an OPENAI_API_KEY with fine-tuning access. No credentials are present in this environment (env has no OPENAI_API_KEY; ~/.config/openai does not exist; openai.OpenAI() construction raises openai.OpenAIError: Missing credentials...). This procedure would submit a real fine-tune, invoke job.cancel() during polling, and confirm the OpenAI dashboard shows the job cancelled and that no InvalidStateError traceback is emitted. The deterministic mocked tests exercise the identical worker state-machine code paths and are the substantive verification of the crash fix; the live test would additionally confirm the remote OpenAI job is cancelled, which is TrainingJobOpenAI.cancel's own responsibility, not the worker guard's.

---

Automatic Fixes PRs can be configured here.

[greptile-apps]

Greptile Summary

This PR attempts to prevent fine-tuning worker threads from writing results to cancelled TrainingJob futures and adds OpenAI regression coverage.

• Guards the success and error completion paths with cancellation checks.
• Adds deterministic tests for normal completion, provider errors, cancellation during polling, and cancellation during upload.
• The guards narrow the cancellation window but do not make the check and result transition atomic.

Confidence Score: 4/5

This PR is not yet safe to merge because concurrent cancellation can still trigger the worker-thread failure it is intended to fix.

The caller and worker operate concurrently, while cancelled() and set_result() acquire the Future's synchronization independently; cancellation between those calls still causes InvalidStateError.

Files Needing Attention: dspy/clients/lm.py

Important Files Changed

Filename	Overview
dspy/clients/lm.py	Adds non-atomic cancellation guards around fine-tune result delivery, leaving a smaller but real cancellation race.
tests/clients/test_openai_provider.py	Adds broad regression coverage, but its synchronization points do not exercise cancellation between the new check and set_result().

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Worker
    participant Future as TrainingJob/Future
    Worker->>Future: cancelled()
    Future-->>Worker: false
    Caller->>Future: cancel()
    Future-->>Caller: transitions to CANCELLED
    Worker->>Future: set_result(...)
    Future-->>Worker: InvalidStateError

Loading

_{Reviews (1): Last reviewed commit: "fix(dspy): guard finetune worker set_res..." | Re-trigger Greptile}

[greptile-apps]

Cancellation race still remains

A caller can cancel the job after cancelled() returns False but before set_result() acquires the Future's lock. Because these are separate operations, set_result() then raises InvalidStateError. This affects both the success path here and the error path at lines 398–399, so the worker thread can still crash in the situation this PR is intended to fix. Completion must handle cancellation atomically or tolerate set_result() losing the race.