Skip to content

fix(dspy): skip FailedPrediction steps in BootstrapFinetune finetune data prep - #87

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-skip-failedprediction-steps-in-bootstrapf-c0af63
Open

fix(dspy): skip FailedPrediction steps in BootstrapFinetune finetune data prep#87
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-skip-failedprediction-steps-in-bootstrapf-c0af63

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 6, 2026

Copy link
Copy Markdown

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

Closes Unknown issue

Bug: BootstrapFinetune.compile crashes with AttributeError: 'FailedPrediction' object has no attribute 'get' whenever any bootstrapped trace step contains an unparseable LM response. bootstrap_trace_data catches AdapterParseError and appends a FailedPrediction as the trace tuple's third element, but BootstrapFinetune._prepare_finetune_data forwarded that FailedPrediction straight into adapter.format_finetune_data, which calls outputs.get(...). Since FailedPrediction has no .get, compilation aborts before fine-tuning even starts — on any model prone to malformed outputs (e.g. small/open models). The existing metric-score filter does not protect against this, because a failed parse still gets a truthy score (format_reward or format_failure_score, e.g. -1), so the failed entry is retained.

Fix: Skip trace steps whose output is a FailedPrediction before calling build_call_data_from_trace / adapter.format_finetune_data in _prepare_finetune_data, mirroring the defensive guards already present in the sibling finetune teleprompters (grpo.py, gepa_utils.py). Failed steps are skipped while successfully-parsed steps are preserved for fine-tuning. This guards against both FailedPrediction-producing paths in bootstrap_trace.py (the AdapterParseError path and the generic-crash capture_crashes path), so BootstrapFinetune is robust regardless of which trace source emits them.

Testing: Verified with DummyLM-based unit and end-to-end tests (the existing test suite uses DummyLM, which always emits adapter-parseable output, so it never reached the failing path — which is why CI missed the bug):

  • Unit tests that a FailedPrediction trace step is skipped while valid steps are preserved, including when a truthy metric score would otherwise retain the failed entry, and with the real ChatAdapter (the actual crash site).
  • An end-to-end compile() test that forces an AdapterParseError during bootstrapping (the reported reproduction) and confirms the run completes instead of crashing.
  • A manual adapter-agnostic check against ChatAdapter and XMLAdapter (both call outputs.get(...)); both now skip the failed step and keep the valid one. (JSONAdapter.format_finetune_data raises NotImplementedError unconditionally and is not a viable BootstrapFinetune adapter regardless of this fix.)
  • The full teleprompt test suite and the CI-equivalent non-extra suite pass with no regressions; ruff check reports only two pre-existing warnings (both in untouched lines of the test file).

Authored by Detail: Automatic Fixes (AI-assisted). I verified the bug by reproducing the crash before the fix and confirming it is resolved after, and I wrote and understand every line of the change and its tests.

✅ Contributor Checklist

  • Pre-Commit checks are passing (locally and remotely)
  • Title of your PR / MR corresponds to the required format (fix(dspy): ..., matching the PR-title regex)
  • Commit message follows required format fix(dspy): skip FailedPrediction steps in BootstrapFinetune finetune data prep

⚠️ Warnings

None. The change is additive and scoped to dspy/teleprompt/bootstrap_finetune.py; the shared bootstrap_trace.py emission logic is untouched, so GRPO/GEPA behavior is unaffected. A separate latent defect — the capture_failed_parses flag in bootstrap_trace.py is declared but never wired into the AdapterParseError handler — is intentionally left out of scope (this fix makes BootstrapFinetune robust regardless of that flag's behavior).


Automatic Fixes PRs can be configured here.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR prevents FailedPrediction objects from being passed to adapter fine-tuning formatters and adds focused preparation and compile-path tests.

  • Valid trace steps are retained while failed parse steps are skipped.
  • Adapter-formatting coverage now includes the real ChatAdapter crash boundary.
  • The all-failed case still schedules an empty fine-tuning dataset, leaving that reported scenario incomplete.

Confidence Score: 4/5

This PR is not yet safe to merge because its all-failed path still launches fine-tuning with no training records and can abort compilation.

The adapter crash is prevented, but when every captured step fails parsing, the replacement path forwards an empty dataset to a real fine-tuning backend; the new compile test passes only because that backend boundary is mocked.

Files Needing Attention: dspy/teleprompt/bootstrap_finetune.py, tests/teleprompt/test_bootstrap_finetune.py

Important Files Changed

Filename Overview
dspy/teleprompt/bootstrap_finetune.py Skips failed trace outputs before adapter formatting, but can consequently submit an empty dataset to fine-tuning.
tests/teleprompt/test_bootstrap_finetune.py Adds useful failed-output coverage, although the all-failed compile test mocks the production boundary that consumes the empty dataset.

Reviews (1): Last reviewed commit: "fix(dspy): skip FailedPrediction steps i..." | Re-trigger Greptile

Comment on lines +180 to +185
if isinstance(trace_instance[2], FailedPrediction):
logger.debug(
"Skipping a trace step with a FailedPrediction output "
"(unparseable LM response) while preparing finetune data."
)
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Empty training data submitted

When every trace step is a FailedPrediction, this guard removes every training record, but compile still passes the empty group to finetune_lms. The real path then calls lm.finetune(train_data=[]); local fine-tuning cannot construct a dataset without samples, and remote providers reject empty training files, so compilation still fails. The new all-failed test does not catch this because it mocks finetune_lms while confirming that the submitted train_data is empty. Empty groups need to be handled before fine-tuning is launched.

Knowledge Base Used:

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.

1 participant