Skip to content

fix(dspy): raise actionable ValueError in KNN when training examples lack with_inputs() - #117

Open
detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-raise-actionable-valueerror-in-knn-when-t-bc0584
Open

detail-app[bot] wants to merge 1 commit into
mainfrom
detail/bug-fix/fix-dspy-raise-actionable-valueerror-in-knn-when-t-bc0584

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: KNN.__init__ serialized each training example with an inline key in example._input_keys filter. Example._input_keys defaults to None until .with_inputs() is called, so constructing KNN from examples that lack .with_inputs() crashed with a cryptic TypeError: argument of type 'NoneType' is not iterable deep in a list comprehension — with no hint that .with_inputs() is required. The KNN docstring example itself omitted .with_inputs(), so users following it hit this immediately.

Fix:

  • Replaced the inline _input_keys access with example.inputs().items(), reusing the framework's existing guard in Example.inputs() (dspy/primitives/example.py), which raises an actionable ValueError("Inputs have not been set for this example. Use \example.with_inputs()` to set them.")when_input_keys is None`. This matches the convention used across the rest of DSPy (bootstrap, evaluate, parallel, etc.) and raises the error during construction, before any embedding work.
  • Updated the KNN docstring example to call .with_inputs("input"), consistent with KNNFewShot's docstring.

Testing:

  • Added two regression tests in tests/predict/test_knn.py (extra marker): one asserts a trainset with no .with_inputs() raises ValueError mentioning with_inputs (not TypeError); the other asserts a mixed trainset (one marked, one unmarked) still raises, confirming the check is per-example.
  • Ran the existing KNN, KNNFewShot, and Example test suites (default + extra): all pass with no regressions.
  • Verified the reproduction end-to-end: unmarked examples now raise the actionable ValueError, while the corrected docstring pattern (.with_inputs("input")) constructs and retrieves neighbors successfully.
  • ruff check and ruff format --check are clean; the Example doctests (--doctest-modules) pass.
  • No typecheck is configured in this repo (no mypy/pyright in pyproject.toml or CI), so that step is not applicable.

✅ Contributor Checklist

  • Pre-Commit checks are passing (locally and remotely)
  • Title of your PR / MR corresponds to the required format
  • Commit message follows required format {label}(dspy): {message}

⚠️ Warnings

  • AI-assisted contribution: Authored by Detail (automatic fix). The bug was reproduced from the report, the fix reuses the existing Example.inputs() guard, and all verification listed above was run locally.

Automatic Fixes PRs can be configured here.

@greptile-apps

greptile-apps Bot commented Sep 6, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes KNN initialization use the validated Example.inputs() accessor, producing an actionable ValueError when training examples have not declared inputs.

  • Corrects the KNN usage example to call with_inputs("input").
  • Adds regression coverage for wholly unmarked and mixed trainsets.
  • Preserves serialization behavior for ordinary, correctly marked input fields.

Confidence Score: 5/5

The PR appears safe to merge, with the intended validation behavior covered by focused regression tests.

The public accessor preserves existing serialization for ordinary marked inputs while replacing the previous cryptic failure with the documented actionable error, and no concrete regression remains.

Important Files Changed

Filename Overview
dspy/predict/knn.py Replaces direct _input_keys filtering with the guarded public accessor and corrects the documented construction pattern.
tests/predict/test_knn.py Adds focused regression tests confirming unmarked examples raise an actionable ValueError.

Reviews (1): Last reviewed commit: "fix(dspy): raise actionable ValueError i..." | Re-trigger Greptile

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