Skip to content

feat(rig-core): return conversation messages from non-streaming agent…#1450

Merged
joshua-mo-143 merged 1 commit into0xPlaygrounds:mainfrom
illegalcall:feat/1445-prompt-response-messages
Mar 3, 2026
Merged

feat(rig-core): return conversation messages from non-streaming agent…#1450
joshua-mo-143 merged 1 commit into0xPlaygrounds:mainfrom
illegalcall:feat/1445-prompt-response-messages

Conversation

@illegalcall
Copy link
Copy Markdown
Contributor

Summary

  • Adds messages: Option<Vec<Message>> field to PromptResponse (with #[non_exhaustive])
  • Populated only via extended_details() path — Standard path pays zero cost
  • Internal collect_messages bool + private into_extended() avoids unnecessary chat_history.to_vec() on Standard delegation
  • Mirrors existing streaming pattern (FinalResponse::history)

Changes

  • rig-core/src/agent/prompt_request/mod.rs: collect_messages field, conditional message collection, PromptResponse::with_messages() builder
  • rig-core/tests/prompt_response_messages.rs: 10 integration tests (mock models, multi-turn, backward compat, error paths)

Test plan

  • cargo test -p rig-core --lib --tests (396 passed)
  • cargo check --workspace (clean)
  • Standard path backward compat (returns String, no perf regression)
  • extended_details() populates messages (single-turn + multi-turn with tool calls)
  • with_history() + extended_details() interop
  • MaxTurnsError path unaffected
  • PromptResponse::new() 2-arg signature preserved

Copy link
Copy Markdown
Contributor

@joshua-mo-143 joshua-mo-143 left a comment

Choose a reason for hiding this comment

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

please see comments

hook: self.hook,
concurrency: self.concurrency,
output_schema: self.output_schema,
collect_messages: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't appear correct, collect_messages shouldn't automatically become false here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hook: self.hook,
concurrency: self.concurrency,
output_schema: self.output_schema,
collect_messages: true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should not be arbitrarily set to true

Comment on lines +91 to +92
/// Whether to collect message history in the response
collect_messages: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Needs a method to set to true/false

Comment thread rig/rig-core/src/agent/prompt_request/mod.rs
@illegalcall illegalcall force-pushed the feat/1445-prompt-response-messages branch from 3d8f441 to 516e747 Compare March 1, 2026 13:29
@illegalcall
Copy link
Copy Markdown
Contributor Author

Thanks for the review! I believe with_hook() was actually preserving via self.collect_messages here, but agreed the overall collect_messages design was confusing.

I've simplified it, removed the bool entirely, Extended path now always populates messages.
The to_vec() cost is negligible vs LLM latency and this should resolve comments 2 and 3 as well.
Also added the Display impl.

Please lmk if this looks good now. Thank you :)

Copy link
Copy Markdown
Contributor

@joshua-mo-143 joshua-mo-143 left a comment

Choose a reason for hiding this comment

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

lgtm

… loop

Add `messages: Option<Vec<Message>>` field to `PromptResponse` so that
`extended_details()` returns the full message history accumulated during
the agent loop, without requiring callers to pass `&mut Vec<Message>`
via `with_history()`.

Changes:
- Add `messages` field to `PromptResponse` with `#[non_exhaustive]`
- Add `Display` impl on `PromptResponse` delegating to `output`
- Add `with_messages()` builder method on `PromptResponse`
- Always populate messages in the Extended send path
- Standard path remains unchanged (returns plain `String`)
- Add 11 integration tests with mock models covering single-turn,
  multi-turn with tool calls, backward compat, and error paths

Fixes 0xPlaygrounds#1445
@illegalcall illegalcall force-pushed the feat/1445-prompt-response-messages branch from 516e747 to 18f662f Compare March 2, 2026 23:21
@illegalcall
Copy link
Copy Markdown
Contributor Author

Fixed merge conflicts.

@joshua-mo-143 joshua-mo-143 added this pull request to the merge queue Mar 3, 2026
Merged via the queue into 0xPlaygrounds:main with commit 53471db Mar 3, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request Feb 28, 2026
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