fix: do not post_process before finally in ModelOutputThunk.astream#580
fix: do not post_process before finally in ModelOutputThunk.astream#580psschwei merged 4 commits intogenerative-computing:mainfrom
Conversation
Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>
|
The PR description has been updated. Please fill out the template for your PR to be reviewed. |
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
|
@nrfulton feel free to treat this with full skepticism |
|
I think this looks good and is a return to what we had before. Let's do a full clean install and test run to make sure the changes to core didn't mess anything up. Also, I believe the reason the always post_process change was made was to accommodate telemetry span closure, we should make sure this doesn't cause exceptions / errors there. |
Signed-off-by: Paul S. Schweigert <paul@paulschweigert.com>
|
Well, trying to run the tests crashed my laptop, so that's not promising... need to look into why |
|
I ran the vllm tests remotely and they all passed. Well, technically I ran full logs: |
|
will run ollama locally now |
|
Ollama tests pass, except for the decompose test that should be fixed in #569 |
|
This also closes #432 which I observed in testing |
jakelorocco
left a comment
There was a problem hiding this comment.
lgtm; tests pass and potential issues with telemetry spans were addressed
Misc PR
Type of PR
Description
Link to Issue: Do not
post_processbeforefinallyinModelOutputThunk.astream#577Removed the finally block in ModelOutputThunk.astream that called post_process even when an exception occurred during generation
Exceptions from generation now propagate directly instead of being swallowed by secondary failures in post_process
post_process and output parsing now only run on the successful completion path
Testing