feat: added support for streaming in API

[ErikBjare]

It's kinda working with the basic UI.

Been struggling to get it working for https://github.com/ErikBjare/gptme-webui-demo/, but almost there!

---

Important

Adds streaming support to the API for real-time token streaming and updates client-side code to handle and display streamed data.

• API Changes:
  • api_conversation_generate() in api.py now supports streaming responses using Server-Sent Events (SSE).
  • Implements token streaming from _stream() and yields each token as a JSON event.
  • Handles tool execution and error logging during streaming.
• Client-Side Changes:
  • generate() in main.js updated to handle SSE, reading streamed tokens and updating the chat log in real-time.
  • Adds error handling for streaming failures and updates UI accordingly.
• Tests:
  • Adds test_api_conversation_generate_stream() in test_server.py to test streaming responses.
  • Updates test_api_conversation_generate() to test non-streaming responses.

^{This description was created by for 31456ea. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 8e1a4cd in 1 minute and 32 seconds

More details

• Looked at 212 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. gptme/server/api.py:139

• Draft comment:
  Ensure that _stream does not return empty chunks to avoid unnecessary iterations or infinite loops.
• Reason this comment was not posted:
  Comment did not seem useful.

2. gptme/server/static/main.js:247

• Draft comment:
  Consider adding currentMessage to chatLog only after a successful fetch to avoid unnecessary operations.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In the generate method of the Vue instance, the currentMessage object is pushed to chatLog before the fetch request is made. If the fetch request fails, this temporary message is removed, but it might be better to only add it after a successful fetch to avoid unnecessary operations.

3. gptme/server/static/main.js:310

• Draft comment:
  Ensure UI reflects the generating state change properly in the error block.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In the generate method of the Vue instance, the generating flag is set to false after the fetch request is complete. However, if an error occurs, the flag is also set to false, but the error handling does not ensure that the UI reflects this state change properly. It might be better to handle UI updates more explicitly in the error block.

4. gptme/server/static/main.js:270

• Draft comment:
  Handle cases where value might be undefined or null in the reader.read() loop.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  The comment points out a potential issue with handling undefined or null value in the reader.read() loop. However, the read() method is expected to return a value that is a Uint8Array or undefined when done is true. The code already breaks the loop when done is true, which should cover the case where value is undefined. Handling null value might not be necessary as read() should not return null. The comment might be overly cautious.
  I might be underestimating the potential for value to be null or undefined in unexpected scenarios. There could be edge cases or errors in the stream that result in value being null or undefined, which are not covered by the current code.
  The read() method's contract should ensure that value is a Uint8Array or undefined when done is true. Null values are not expected, and the existing check for done should suffice for handling undefined value.
  The comment is likely unnecessary because the existing code handles the expected cases for value being undefined. The read() method should not return null, and the check for done should cover the undefined case. The comment should be deleted.

Workflow ID: wflow_0z4soavbR8HWWlPU

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 67.30769% with 17 lines in your changes missing coverage. Please review.

Project coverage is 72.50%. Comparing base (381dbc8) to head (31456ea).
Report is 3 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
gptme/server/api.py	67.30%	17 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
- Coverage   73.05%   72.50%   -0.56%     
==========================================
  Files          60       60              
  Lines        3838     3880      +42     
==========================================
+ Hits         2804     2813       +9     
- Misses       1034     1067      +33     

Flag	Coverage Δ
anthropic/claude-3-haiku-20240307	71.52% <67.30%> (-0.32%)	⬇️
openai/gpt-4o-mini	71.41% <67.30%> (-0.08%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ellipsis-dev]

❌ Changes requested. Incremental review on a75adf2 in 57 seconds

More details

• Looked at 129 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. gptme/server/api.py:124

• Draft comment:
  Redundant check for if not msgs:. This check is already performed before the if not stream: block.
• Reason this comment was not posted:
  Confidence changes required: 50%
  In api_conversation_generate, the generate function checks for if not msgs: twice, which is redundant. The first check is already done before the if not stream: block.

Workflow ID: wflow_NJCW05xQC4bcUlvL

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

The test assumes exactly 3 messages are returned, which might not always be true. Consider checking for expected roles and content without assuming the exact number of messages.

Suggested change

	assert len(msgs) == 3 # Assistant message + 2 system messages from tool output
	assert all(msg['role'] in ['assistant', 'system'] for msg in msgs)

[ellipsis-dev]

The test only checks for non-empty data but does not validate the content of the streamed data. Consider checking for expected roles and content in the streamed data.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 31456ea in 20 seconds

More details

• Looked at 29 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. tests/test_server.py:88

• Draft comment:
  Consider adding assertions to validate the content of the response, not just its presence. This will ensure the API is returning the expected data.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The test for non-streaming responses has been updated, but it lacks validation of the actual content of the response.

Workflow ID: wflow_OMruJW8fVFwTPTsi

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.