Skip to content
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

Add tests for json_utils.json_fix_llm #2952

Merged
merged 38 commits into from
Apr 23, 2023
Merged

Add tests for json_utils.json_fix_llm #2952

merged 38 commits into from
Apr 23, 2023

Conversation

coditamar
Copy link
Contributor

Background

I get the following error a lot. Before debugging and refactoring, let's add relevant simple unit-tests.
Error: The following AI output couldn't be converted to a JSON:

Changes

Added tests for json_utils.json_fix_llm::fix_json_using_multiple_techniques()

Documentation

Test Plan

Add tests, they pass

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

coditamar and others added 24 commits April 21, 2023 11:23
@coditamar
config.py: make load_dotenv(override=True)
a71e65d
@waynehamadi
Update Python version and benchmark file in benchmark.yml
c83667e
@waynehamadi
Refactor main function and imports in cli.py
f4db77a
@waynehamadi
Update import statement in ai_config.py
81d6ae4
@waynehamadi
Add set_temperature and set_memory_backend methods in config.py
59c28fb
@waynehamadi
Remove unused import in prompt.py
296fab3
@waynehamadi
Add goal oriented tasks workflow
ca3a441
@waynehamadi
Added agent_utils to create agent
5653b52
@waynehamadi
added pytest and vcrpy
3327e1f
@waynehamadi
added write file cassette
4a339e1
@waynehamadi
created goal oriented task write file with cassettes to not pay opena…
9cc126c 
…i tokens
@waynehamadi
solve conflicts
35e91a0
@waynehamadi
add ability set azure because github workflow needs it off
712a449
@waynehamadi
solve conflicts in cli.py
af0fef1
@waynehamadi
black because linter fails
11234de
@waynehamadi
solve conflict
baaf4d1
@waynehamadi
setup github action to v3
fb5e916 
Signed-off-by: Merwane Hamadi <[email protected]>
@waynehamadi
fix conflicts
fdbfa7a 
Signed-off-by: Merwane Hamadi <[email protected]>
@richbeales
Merge pull request Significant-Gravitas#2461 from merwanehamadi/featu…
8793f67 
…re/create-smoke-test

Feature/create smoke test
@ntindle
Merge pull request Significant-Gravitas#2931 from riensen/fix/multipl…
61600b7 
…e-plugins

Enable support for loading multiple plugins per zip file
@richbeales
Plugins: debug line always printed in plugin load
5b06290
@BillSchumacher
Merge pull request Significant-Gravitas#2936 from Significant-Gravita…
11dd1ef 
…s/richbeales-patch-1

Plugins: debug line always printed in plugin load
@coditamar
Merge remote-tracking branch 'upstream/master'
eea4267
@coditamar
add test for fix_json_using_multiple_techniques
7257e53
@ntindle
Copy link
Member

ntindle commented Apr 22, 2023

Run the linter please

@ntindle
Copy link
Member

ntindle commented Apr 22, 2023

Also let’s add some more realistic test cases for the content we expect to see

@ntindle
Copy link
Member

ntindle commented Apr 22, 2023

Not sure why this needs the api key. Will look a bit later today when I’m at my laptop

@codecov
Copy link

codecov bot commented Apr 22, 2023 •
edited
Loading

Codecov Report

Patch coverage has no change and project coverage change: -8.16 ⚠️

Comparison is base (d6ef9d1) 46.75% compared to head (7051cd2) 38.59%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2952      +/-   ##
==========================================
- Coverage   46.75%   38.59%   -8.16%     
==========================================
  Files          63       63              
  Lines        3016     3016              
  Branches      496      496              
==========================================
- Hits         1410     1164     -246     
- Misses       1493     1786     +293     
+ Partials      113       66      -47

see 16 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@coditamar
Copy link
Contributor Author

Also let’s add some more realistic test cases for the content we expect to see

I intentionally wanted to keep it very simple, to start lean. And as you can see, we already find interesting behavior,
that required using mocks

ntindle
ntindle requested changes Apr 22, 2023
View reviewed changes
Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

Wait to mkae the change mentioned until I discuss with team

tests/unit/test_json_utils_llm.py Show resolved Hide resolved
tests/unit/test_json_utils_llm.py Show resolved Hide resolved
tests/unit/test_json_utils_llm.py Show resolved Hide resolved
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 22, 2023
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Apr 22, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@richbeales richbeales requested a review from ntindle April 23, 2023 06:28
ntindle
ntindle requested changes Apr 23, 2023
View reviewed changes
Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

Fix up merge artifacts and it’s 🚀

.github/workflows/goal_oriented_tasks.yml Outdated Show resolved Hide resolved
tests/integration/goal_oriented/test_write_file.py Outdated Show resolved Hide resolved
tests/unit/test_json_utils_llm.py Show resolved Hide resolved
@coditamar
Copy link
Contributor Author

coditamar commented Apr 23, 2023 •
edited
Loading

Two unit-tests failures seems unrelated to the PR

@coditamar coditamar requested a review from ntindle April 23, 2023 12:50
ntindle
ntindle requested changes Apr 23, 2023
View reviewed changes
tests/unit/test_json_utils_llm.py Outdated Show resolved Hide resolved
tests/unit/test_json_utils_llm.py Show resolved Hide resolved
@ntindle ntindle merged commit ec71075 into Significant-Gravitas:master Apr 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntindle ntindle ntindle approved these changes

Assignees
No one assigned
Labels
size/l testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@coditamar @ntindle @waynehamadi @richbeales @BillSchumacher