Skip to content

Fix dspy task 8635: change min_instr_chars default from 30 to 0#40

Open
AlienKevin wants to merge 1 commit intocooperbench:mainfrom
AlienKevin:fix/dspy-t8635-min-instr-chars
Open

Fix dspy task 8635: change min_instr_chars default from 30 to 0#40
AlienKevin wants to merge 1 commit intocooperbench:mainfrom
AlienKevin:fix/dspy-t8635-min-instr-chars

Conversation

@AlienKevin
Copy link
Contributor

Summary

  • The combined.patch for dspy task 8635 adds instruction length bounds to GenerateModuleInstruction and GroundedProposer with min_instr_chars=30
  • The pre-existing test test_propose_instructions_for_program[None] uses DummyLM({"proposed_instruction": "instruction"}) which returns "instruction" (11 chars)
  • This falls below the 30-char minimum, causing the code to fall back to the default signature instruction ("Given the fields 'question', produce the fields 'answer'.")
  • The assertion pred_instructions == ["instruction"] then fails
  • tests2.patch already corrects this assertion, but tests1.patch does not — so feature 1 tests always fail when run against the combined implementation

Fix

Change min_instr_chars default from 30 to 0 in both GenerateModuleInstruction.__init__ and GroundedProposer.__init__ within combined.patch. The length-bounds feature remains fully functional when explicitly configured, but the default no longer silently rejects short mock/test instructions.

Test plan

  • Oracle test passes for dspy task 8635 (both features)
  • Verified on Modal with Harbor evaluation framework

The combined.patch adds instruction length bounds with min_instr_chars=30.
The pre-existing test test_propose_instructions_for_program[None] uses
DummyLM which returns "instruction" (11 chars). This falls below the
30-char minimum, causing the code to fall back to the default signature
instruction, breaking the assertion.

tests2.patch corrects this assertion but tests1.patch does not, so
feature 1 tests always fail.

Changing the default to 0 preserves backward compatibility while keeping
the length-bounds feature functional when explicitly configured.
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