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.

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

Release 0.14.10 #224

Merged
merged 19 commits into from
Oct 11, 2024
Merged

Release 0.14.10 #224

merged 19 commits into from
Oct 11, 2024

Conversation

apepkuss
Copy link
Collaborator

Major changes:

  • chat-prompts crate

  • endpoints crate

    • Extend ImageCreateRequest and ImageEditRequest for better support for stable-diffusion models

…Prompt`

Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Signed-off-by: Xin Liu <sam@secondstate.io>
Copy link
Contributor

juntao commented Oct 11, 2024

Hello, I am a code review agent on flows.network. Here are my reviews of changed source code files in this PR.


Cargo.toml

Potential issues

  1. "crates/endpoints" and "crates/chat-prompts" versions are not consistent with other crates; the patching mechanism should be reviewed.
  2. "serde_json": "1.0" version might not have all features included which can cause compatibility issues.
  3. The patch for reqwest, hyper, and tokio don't match their respective git branches. It is recommended to use tags or commit hashes instead of branches in patching mechanism.

Summary of changes

  1. Updated chat-prompts version from 0.16 to 0.17.
  2. Added features = ["derive"] for the serde crate.
  3. Included cargo and derive features in clap crate.

crates/chat-prompts/Cargo.toml

Potential issues

  1. version = "0.17.0": Ensure this aligns with the actual version of your software for consistency.
  2. edition = "2021": Make sure to support all features and compatibility issues related to Rust's new edition.
  3. license = "Apache-2.0": Check that this matches with the actual license applied for your software.

Summary of changes

  1. Version Update: The version number has been updated from 0.16.1 to 0.17.0, indicating a new release or update of the package "chat-prompts".
  2. Dependency Addition: A new dependency 'tera' was added with the version 1.12. It could be used for rendering templates.
  3. Repository Update: The repository address has been updated to point towards a GitHub repo called LlamaEdge/LlamaEdge.

crates/chat-prompts/src/chat/functionary.rs

Potential issues

  1. The build method in both FunctionaryV32ToolPrompt and FunctionaryV31ToolPrompt is unimplemented, indicating that this function needs to be implemented but hasn't been done yet.
  2. In the create_system_prompt function, there are potential issues with the use of unwrap(). The code assumes that these methods will always return a value; however, they might not if an error occurs. This could cause the program to panic at runtime.
  3. The Tera template rendering in create_system_prompt is embedded directly into the Rust source code as a string. While this works for small applications, it can be problematic for larger projects with more complex templates or layouts. It's recommended that Tera templates are stored separately from the application logic.

Summary of changes

  1. Implemented a new BuildChatPrompt trait with two methods: build() and build_with_tools().
  2. Created the classes for "FunctionaryV32ToolPrompt" (for release 0.14.10), which handles the system prompt creation using Tera templates, and appending user/assistant messages.
  3. Similarly created a class for "FunctionaryV31ToolPrompt". The difference being it uses IPython environment and doesn't use any template engine but constructs prompts manually instead.

crates/chat-prompts/src/chat/mod.rs

Potential issues

  1. The code contains many unused imports at the top, which could cause unnecessary overhead and confusion among developers.
  2. There is a method build_with_tools defined in the trait BuildChatPrompt but it doesn't do anything different from its default implementation (build). This seems like an oversight that could be corrected.
  3. Some models or prompts are not properly linked with the From conversion, leading to panic when used. Correct these cases and provide proper linkages for all models and prompts.

Summary of changes

:

  1. Added a new module "functionary" - This appears to be the addition of a new tool prompt in the Functionary version 32 and 31.
    These versions might include some new functionalities or tools not present in previous versions.
  2. Modified the enum ChatPrompt - The updated file now contains an additional variant for handling functionary v32 tool prompts.
  3. Implemented From for ChatPrompt - This indicates that we can convert a template type into different chat prompt types, including FunctionaryV32ToolPrompt.

crates/chat-prompts/src/lib.rs

Potential issues

The file defines an enumeration for the different prompt types, as well as a series of helper functions to work with these templates.

One major issue in this code is the absence of any comments or documentation explaining what certain parts of the code do. This makes it difficult to understand and maintain the code later on.

Another issue could be the MergeRagContext trait, which appears to have a default implementation that only handles the SystemMessage case when there's a system prompt. If the chat template doesn't have a system message, this function does nothing, leading to potential bugs if it is not handled properly elsewhere in the codebase.

Moreover, the FromStr implementation could potentially panic due to unknown input string causing an unhandled case in the match expression which results in no value for the Ok variant of the Result returned by from_str() method.

Finally, there seems to be some inconsistency around how system messages are handled and formatted within the different prompt types, particularly given that some prompts have specific ways they handle system messages while others don't. This could potentially cause unexpected behavior if not properly implemented for all cases.

Summary of changes

  1. Added FunctionaryV32 and FunctionaryV31 as a new variant in the PromptTemplateType enum. These variants could be used to generate prompts related to these specific versions of functionaries.
  2. Updated the implementation of PromptTemplateType to include cases for FunctionaryV32 and FunctionaryV31. This will help to handle these new variants properly in different parts of code where template types are utilized.
  3. In the FromStr implementation, added corresponding entries for FunctionaryV32 and FunctionaryV31 that correctly map string inputs into their respective variant values from PromptTemplateType enum.

crates/endpoints/Cargo.toml

Potential issues

  1. Incorrect Version: The indexmap dependency version is out of sync with the workspace, causing potential compatibility issues during compilation or runtime.
  2. Dependency Issue: Serde and serde_json workspace dependencies might be leading to inconsistencies in different parts of your code base due to varying versions across different packages.
  3. URL Version: The url version is not consistent between the dependencies and can cause potential bugs if the package is updated or modified by external actors.

Summary of changes

  1. Version Update: The package version is updated from "0.15.0" to "0.15.1".
  2. Bug Fixing: This change patch likely contains a fix for an issue in the previous version, hence the increment in the minor version number (1 instead of 0).
  3. Maintenance: The package details and repository information are updated without introducing any significant functional changes to the software component.

crates/endpoints/src/audio/speech.rs

Potential issues

The provided source code is a part of a larger Rust program. Here are some issues highlighted in the provided snippet:

  1. The "model" field in SpeechRequest struct must always be present. However, it is not enforced during deserialization which might lead to invalid state if an incomplete JSON object is received.
  2. The "input" field in SpeechRequest struct also needs similar validation as the model. It should always be required and not allowed missing from a JSON object.
  3. The response_format and speed fields are incorrectly initialized to Some(SpeechFormat::Wav) and Some(1.0) respectively when they're not present in the JSON input. However, it would be more efficient and accurate if these defaults were applied directly at the field definition level rather than during deserialization.

Summary of changes

  • Updated the implementation to make it compatible with Deserialize 0.14.10.
  • Changed the 'de' lifetime parameter in Visitor implementation from a fixed version to dynamic lifetime (_) as per new release guidelines.

crates/endpoints/src/audio/translation.rs

Potential issues

  1. FileObject must be properly implemented: It seems like it's not fully defined, which might cause issues with the file handling in this code snippet.
  2. Default language and response format settings: The default values for response_format (set to "json") and temperature (set to 0.0) could lead to unexpected behaviour if these fields are not explicitly specified by the user.
  3. Duplicate and missing field errors: It's good practice to check for duplicate and missing field errors during deserialization, but in this case it seems like there is a potential bug where those checks are not properly implemented.

Summary of changes

  • Updated the Visitor to work with the new 'de' lifetime
  • Improved error handling in field visitor
  • Refactored for better struct organization and cleaner code.

crates/endpoints/src/images.rs

Potential issues

It appears there is a cut-off in the middle of the prompt. I'm unable to provide a full analysis or review at this time. However, here are some issues that might be present:

  1. Serialization error in deserialize method for ImageEditRequest struct - The serialized fields do not match with the fields defined in the structure.
  2. Incorrect handling of optional values - For optional fields like mask, n, size, etc. there should be proper checking and handling if they are none.
  3. No tests for error scenarios - Code does not contain any test cases to handle erroneous inputs or exceptions.
  4. Lack of input validation - There seems no validation check on the size of image (height & width), model name, or file object's content type and size, etc. These checks can prevent potential security vulnerabilities like code injection or denial-of-service attacks.

Summary of changes

:
The following is a summary of changes made in the patch file. Key changes listed with top 3 most important ones.

  1. Addition of new fields to the ImageCreateRequest and ImageEditRequest: 'cfg_scale', 'sample_method', 'steps', 'height', 'width', 'control_strength', 'seed', 'strength'.
  2. Default values for these fields are set in their respective structures and added default constructor to ImageCreateRequest and ImageEditRequest.
  3. Handling of duplicate field error during deserialization updated accordingly.
  4. Serialization test case updated with the newly added fields.
  5. Updated serialize and deserialize tests for ImageVariationRequest.
  6. Updated formatting in code comments as per linter rules.
  7. Implemented new methods to set optional parameters in ImageCreateRequestBuilder and ImageEditRequestBuilder.

crates/llama-core/Cargo.toml

Potential issues

  1. The readme field in the [package] section should point to an existing file in the repository, as it is currently set to "README.md" but that doesn't seem like a valid path according to the source code or documentation provided.
  2. The repository field also needs to be accurate with the actual location of your repository containing the source files.
  3. The version of wasmedge-wasi-nn, uuid and either dependencies are mentioned as workspace = true but these aren't defined in any workspace configuration. Hence, it will result in an error during build.

Summary of changes

  1. Updated package version from 0.19.2 to 0.19.3
  2. Included git repository for wasmedge_stable_diffusion package. This means a specific branch of the repository will be used instead of a fixed version number.
  3. Added base64 and cargo-machete as workspace dependencies, probably these are necessary tools or libraries for this project.

crates/llama-core/src/chat.rs

Potential issues

:
created: created.as_secs(),
model: graph.name().to_owned(),
choices: vec![ChatCompletionObjectChoice {
index: 0,
message: ChatCompletionObjectMessage {
role: ChatCompletionRole::Assistant,
content: Some(message),
function_call: None,
},
finish_reason: if message.is_empty() { FinishReason::stop } else { FinishReason::length },
logprobs: None,
}],
usage: Usage {
prompt_tokens: token_info.prompt_tokens,
completion_tokens: token_info.completion_tokens,
total_tokens: token_info.prompt_tokens + token_info.completion_tokens,
},
})
}
}
}
Err(wasmedge_wasi_nn::Error::BackendError(err) @ wasmedge_wasi_nn::BackendError::Compute(msg)) => {
let err_msg = format!("Failed to compute the chat completion. Reason: {}.", msg);

        #[cfg(feature = "logging")]
        error!(target: "stdout", "{}", &err_msg);

        Err(LlamaCoreError::Backend(BackendError::Compute(err_msg)))
    }
}

}

Summary of changes

:

  1. Added support for FunctionaryV32 and FunctionaryV31 prompt templates, enhancing tool usage functionality.
  2. Modified the system message handling in build_prompt function to fit into updated system messages sequence.
  3. Adjusted token validation in build_prompt function to accommodate different system message structures.

crates/llama-core/src/images.rs

Potential issues

The following issues have been identified in this code:

  1. The variable ctx is not being used after the assignment of the lock acquired from the SD_IMAGE_TO_IMAGE reader's lock, which could potentially lead to a deadlock situation as the lock isn't being held in any subsequent operations.
  2. The method set_image() seems to take ownership of the image file path string provided but doesn't actually use it until the call to generate(). This might be prone to runtime errors if the original string gets dropped before then.
  3. In the function image_to_base64, the read_to_end() function is called on a file that hasn't been fully read yet. This could result in an incorrect or truncated byte array being returned by this method, and hence lead to invalid base64 string generation.
  4. There seems to be a lot of repetitive logging code, especially in the methods image_generation(), image_edit() and set_prompt(). This can potentially be improved with some refactoring.

Summary of changes

:

  1. Modified the set of prompts, now using the prompt from req and setting it for the context.
  2. Added configuration parameters such as n (number of images to generate), cfg_scale, sample_method, steps (sampling steps), height, width, control_strength, seed, and strength.
  3. Updated the image generation process by setting new parameters on the context: batch_count, sample_method, sample_steps, height, width, control_strength, seed, and strength.
  4. Finalized the logging of the updated information for debugging purposes.

llama-api-server/Cargo.toml

Potential issues

  1. The use of workspace = true for dependencies can lead to version conflicts if different versions are required in the future. It is better to pin each package explicitly to a specific version.
  2. The "edition" field should be set as "2021", but this could introduce compatibility issues with Rust 2015 or earlier.
  3. Hyper, futures-util and multipart-2021 are using the feature "full" which might lead to unnecessary bloat in terms of binary size due to its extensive features. It would be good to specify only necessary features for each dependency.

Summary of changes

  • Updated the version of this package from 0.14.9 to 0.14.10.

llama-chat/Cargo.toml

Potential issues

  1. The package name "llama-chat" does not follow standard naming conventions and might cause confusion when working on large teams or projects.
  2. All the dependencies are set to workspace = true, which could lead to potential compatibility issues if different versions of these libraries are used in different parts of your codebase.
  3. The version number "0.14.10" does not adhere to semantic versioning standards (major.minor.patch) and can cause confusion with future updates or releases.

Summary of changes

:

  1. Version Update: The package version is updated from 0.14.9 to 0.14.10, indicating a new release or update in the "Release 0.14.10".
  2. [No other major change was observed]

llama-simple/Cargo.toml

Potential issues

  1. "wasmedge-wasi-nn", "clap", "once_cell", and dependencies should be specified clearly in the code so that users can understand what dependencies they need to install for the package.
  2. The version of these dependencies (like "workspace = true") is not clear; it would be helpful if this was indicated more specifically.
  3. It seems like there's an attempt made here to use workspace but no actual workspaces are declared in the root 'Cargo.toml'; thus, this could lead to potential issues or confusion for developers.

Summary of changes

:

  • Updated version from 0.14.9 to 0.14.10 for the package.

@apepkuss apepkuss merged commit cb13c66 into main Oct 11, 2024
8 checks passed
@apepkuss apepkuss deleted the release-0.14.10 branch October 11, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants