Skip to content

Fix instruction_first kwarg silently dropped in (a)run_instructor_with_payload - #827

Draft
Osamaali313 wants to merge 1 commit into
HumanSignal:masterfrom
Osamaali313:fix/messagesbuilder-instruction-first-kwarg
Draft

Fix instruction_first kwarg silently dropped in (a)run_instructor_with_payload#827
Osamaali313 wants to merge 1 commit into
HumanSignal:masterfrom
Osamaali313:fix/messagesbuilder-instruction-first-kwarg

Conversation

@Osamaali313

Copy link
Copy Markdown

Problem

run_instructor_with_payload and arun_instructor_with_payload (adala/utils/llm_utils.py) build a MessagesBuilder with the kwarg instructions_first=:

messages_builder = MessagesBuilder(
    user_prompt_template=user_prompt_template,
    system_prompt=instructions_template,
    instructions_first=instructions_first,   # wrong name
    ...
)

But MessagesBuilder's field is instruction_first (no trailing "s"):

# adala/utils/message_builder.py
instruction_first: bool = Field(default=True)
...
if self.instruction_first:
    # Add system message as first message
    messages.insert(0, {"role": "system", "content": self.system_prompt})

MessagesBuilder is a pydantic model, so under pydantic's default extra="ignore" the misspelled instructions_first= kwarg is silently discarded, and instruction_first keeps its default True. The caller's value has no effect.

Impact

run_instructor_with_payload is called by LiteLLMChatRuntime.record_to_record (adala/runtimes/_litellm.py), which forwards instructions_first (its own default is False). Because the kwarg is dropped, the instruction/system message is always placed first regardless of what the runtime requests. arun_instructor_with_payload is an exported async util with the same defect.

Evidence (sibling proves intent)

The two plural siblings in the same module already pass the correct field name:

# run_instructor_with_payloads / arun_instructor_with_payloads
instruction_first=instructions_first,

Only the two singular functions use the typo.

Reproduction

Caller requests instructions_first=False; observe message roles:

roles produced
before (instructions_first=) ['system', 'user'] — instruction forced first ❌
after (instruction_first=) ['user'] — request honored ✅

Fix

Use the correct field name in both functions:

instruction_first=instructions_first,

`run_instructor_with_payload` and `arun_instructor_with_payload` construct
a `MessagesBuilder` with `instructions_first=instructions_first`, but
`MessagesBuilder`'s field is named `instruction_first` (no trailing "s"):

    # utils/message_builder.py
    instruction_first: bool = Field(default=True)
    ...
    if self.instruction_first:

`MessagesBuilder` is a pydantic model, so under the default `extra="ignore"`
the misspelled `instructions_first=` kwarg is silently discarded and
`instruction_first` stays at its default `True`. The caller's value never
takes effect — e.g. `LiteLLMChatRuntime.record_to_record`
(runtimes/_litellm.py) passes `instructions_first=False`, but the
instruction/system message is still placed first.

The two sibling functions in the same module,
`run_instructor_with_payloads` and `arun_instructor_with_payloads`, already
use the correct field name `instruction_first=instructions_first`. Align
the two singular functions with them.
Copilot AI review requested due to automatic review settings July 16, 2026 21:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@robot-ci-heartex
robot-ci-heartex marked this pull request as draft July 17, 2026 07:42
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.

2 participants