Skip to content

Writer model provider #228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 20 commits into from
Jul 9, 2025
Merged

Writer model provider #228

merged 20 commits into from
Jul 9, 2025

Conversation

yanomaly
Copy link
Contributor

@yanomaly yanomaly commented Jun 16, 2025 •
edited
Loading

Description

Implementation of Writer LLMs provider

Related Issues

#227

Documentation PR

N/A

Type of Change

  • New feature

Testing

Integration and Unit test of created implementation

Checklist

  • I have read the CONTRIBUTING document
  • I have added tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@yanomaly yanomaly changed the title Palmyra providel Palmyra provider Jun 16, 2025
@yanomaly yanomaly marked this pull request as ready for review June 18, 2025 13:41
@awsarron awsarron self-assigned this Jun 25, 2025
awsarron
awsarron requested changes Jun 25, 2025
View reviewed changes
Copy link
Member

@awsarron awsarron left a comment

Choose a reason for hiding this comment

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

Thank you for this awesome contribution @yanomaly!

I added a small number of comments. We'll need to rebase your changes here as well to resolve merge conflicts.

@awsarron
Copy link
Member

would be worth opening a PR for Writer model provider documentation in https://github.com/strands-agents/docs.

https://strandsagents.com/latest/user-guide/concepts/model-providers/llamaapi/ can be used as an example. It would be good to follow the same style, formatting, and language for consistency.

awsarron
awsarron previously requested changes Jun 26, 2025
View reviewed changes
@awsarron awsarron changed the title Palmyra provider Writer model provider Jun 27, 2025
@yanomaly yanomaly requested a review from awsarron June 27, 2025 13:09
@mehtarac mehtarac self-assigned this Jun 27, 2025
@mehtarac mehtarac self-requested a review June 27, 2025 16:23
@sarahcstringer sarahcstringer mentioned this pull request Jul 1, 2025
Merged
6 tasks
@samjulien
Copy link

would be worth opening a PR for Writer model provider documentation in https://github.com/strands-agents/docs.

https://strandsagents.com/latest/user-guide/concepts/model-providers/llamaapi/ can be used as an example. It would be good to follow the same style, formatting, and language for consistency.

PR is here @awsarron strands-agents/docs#115

mehtarac
mehtarac previously approved these changes Jul 8, 2025
View reviewed changes
Copy link
Member

@mehtarac mehtarac left a comment

Choose a reason for hiding this comment

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

tested with

@mehtarac mehtarac dismissed awsarron’s stale review July 8, 2025 23:00

Changes were implemented as requested

@mehtarac mehtarac removed the request for review from awsarron July 8, 2025 23:13
@yonib05
Copy link
Member

yonib05 commented Jul 8, 2025

Hey @yanomaly,

All looks great except it looks like the linter is failing due to this issue:

× No solution found when resolving dependencies: ╰─▶ Because you require ruff>=0.11.6,<0.12.0 and ruff>=0.4.4,<0.5.0, we can conclude that your requirements are unsatisfiable.

Would you mind fixing and we can get this merged?

@yanomaly
Copy link
Contributor Author

yanomaly commented Jul 9, 2025

Hey @yanomaly,

All looks great except it looks like the linter is failing due to this issue:

× No solution found when resolving dependencies: ╰─▶ Because you require ruff>=0.11.6,<0.12.0 and ruff>=0.4.4,<0.5.0, we can conclude that your requirements are unsatisfiable.

Would you mind fixing and we can get this merged?

Great catch @yonib05, thank you! I fixed this issue. Also, I updated provider implementation and tests, to match the latest updates linked with async stream/structured_output methods.

@yanomaly yanomaly requested review from mehtarac and awsarron July 9, 2025 15:22
@yanomaly yanomaly temporarily deployed to manual-approval July 9, 2025 15:24 — with GitHub Actions Inactive
@yonib05 yonib05 merged commit ae01d57 into strands-agents:main Jul 9, 2025
12 checks passed
malte-aws pushed a commit to malte-aws/sdk-python that referenced this pull request Jul 10, 2025
@yanomaly
Writer model provider (strands-agents#228)
10cb4fb 
* feat: palmyra provider initial version

* test: unit tests for Palmyra provider

* feat: message content formatter for different palmyra models

* test: integration tests for palmyra provider

* refactor: changes made by hatch formatter

* refactor: rename filew with Writer name instead of Palmyra

* refactor: change writer provider integration tests structure

* feat: structured outputs for Writer provider

* refactor: add an upper version limit of writer-sdk dependency

* feat: image inputs for Writer models

* refactor: change name of config parameter to specify model from 'model' to 'model_id'

* fix: solve issue for case of empty metadata

* fix: delete unused config arguments

* fix: pyproject.toml fix to pass linters

* fix: fix methods signature to pass updated tests

* feat: implement usage of async Writer client istead of sync

* test: add tests for async agent calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanycoleman ryanycoleman ryanycoleman left review comments

@samjulien samjulien samjulien left review comments

@yonib05 yonib05 yonib05 approved these changes

@mehtarac mehtarac Awaiting requested review from mehtarac

@awsarron awsarron Awaiting requested review from awsarron

Assignees

@awsarron awsarron

@mehtarac mehtarac

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yanomaly @awsarron @samjulien @yonib05 @ryanycoleman @mehtarac