second-state / WasmEdge-WASINN-examples

Apache License 2.0
217 stars 35 forks source link

[Example] ggml: update gemma example for stream output #99

Closed dm4 closed 2 months ago

dm4 commented 2 months ago
juntao commented 2 months ago

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit b0b43c505daa64edf9545aa68cba0ac5ad968462

Key changes in the patch:

  1. Update the main.rs file in the wasmedge-ggml/gemma directory.
  2. Update the example to handle stream output with the addition of conditions for printing "USER:" and "ASSISTANT:" messages.

Potential problems:

  1. The code changes are straightforward, focusing on updating the message prompts for user and assistant. However, the changes might be more meaningful if they were supplemented with some comments explaining the purpose of these updates.
  2. The condition for printing an empty line when the "stream-stdout" option is enabled might cause confusion if not adequately documented. Reviewers could suggest adding comments to clarify the purpose of this condition.
  3. The binary patch in the wasmedge-ggml-gemma.wasm file might need further investigation to ensure that the changes align with the expected code modifications. This could involve running tests to verify the functionality of the changed wasm file.
  4. It's always good practice to ensure that the code formatting and style are consistent with the project's guidelines. Reviewers could check if the new code follows the project's style guide.