Skip to content

Conversation

sjrl
Copy link
Contributor

@sjrl sjrl commented Apr 8, 2025

Related Issues

Proposed Changes:

How did you test it?

  • Added tests
  • Also added more tests for State

Notes for the reviewer

Most of the line changes come from adding tests.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@coveralls
Copy link
Collaborator

coveralls commented Apr 8, 2025

Pull Request Test Coverage Report for Build 14373748478

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 6 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.03%) to 90.378%

Files with Coverage Reduction New Missed Lines %
dataclasses/state.py 1 98.25%
components/agents/agent.py 5 95.24%
Totals Coverage Status
Change from base Build 14373744615: 0.03%
Covered Lines: 10670
Relevant Lines: 11806

💛 - Coveralls

@sjrl sjrl marked this pull request as ready for review April 9, 2025 06:12
@sjrl sjrl requested review from a team as code owners April 9, 2025 06:12
@sjrl sjrl requested review from dfokina and mpangrazzi and removed request for a team April 9, 2025 06:13
@github-actions github-actions bot added the type:documentation Improvements on the docs label Apr 9, 2025
Copy link
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

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

LGTM! One thing (but you're probably aware of): modifying a mutable object obtained via get() can affect the internal state directly.

Example:

def test_state_mutability():
    state = State({"my_list": {"type": list}}, {"my_list": [1, 2]})

    my_list = state.get("my_list")
    my_list.append(3)

    assert state.get("my_list") == [1, 2, 3]

Feel free to ignore if you don't think this can possibly be a bug ;)

@sjrl sjrl enabled auto-merge (squash) April 9, 2025 13:30
@sjrl sjrl merged commit 7bb9c69 into main Apr 10, 2025
17 checks passed
@sjrl sjrl deleted the fix-agents-messages branch April 10, 2025 06:39
sjrl added a commit that referenced this pull request Apr 10, 2025
* Fix issue and set messages in state_schema at init time

* Add reno

* Small changes and add more tests for state

* Add comment

* PR comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agent component doesn't have an output called messages
3 participants