Update ChatCompletion to Mistral API version 0.0.2

[Dannyj1]

Large parts of ChatCompletionRequest were rewritten to support certain changes in the Mistral API. See the PR summary and the MIGRATION.md for more information.

---

Important

Update ChatCompletion to Mistral API version 0.0.2 with refactored message handling, multi-modal content support, and updated dependencies.

• Behavior:
  • Refactor ChatCompletionRequest to use List<ChatMessage> instead of List<Message>.
  • Introduce ChatMessage as an abstract base class with subclasses SystemMessage, UserMessage, AssistantMessage, ToolMessage.
  • Add support for multi-modal content using ContentChunk interface with implementations like TextChunk, ImageURLChunk.
  • Add ToolCall support in AssistantMessage and ToolMessage for function calling.
  • Update DeltaChoice to include DeltaMessage for streaming responses.
• Breaking Changes:
  • Remove Message class and logProbs field from Choice.
  • Change finishReason type in Choice to FinishReason enum.
  • Remove encodingFormat from EmbeddingRequest.
  • Remove ModelPermission class.
• Models:
  • Add ModelCapabilities to provide model capability information.
• Misc:
  • Update dependencies: jackson-databind to 2.18.3, jakarta.validation-api to 3.1.1, okio to 3.11.0.
  • Increase default read timeout to 120 seconds.
  • Update GitHub Actions workflows for SonarQube and CodeQL.

^{This description was created by for ebc2d74. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to d31115f in 3 minutes and 37 seconds. Click for details.

• Reviewed 3968 lines of code in 50 files
• Skipped 0 files when reviewing.
• Skipped posting 28 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/main/java/nl/dannyj/mistral/MistralClient.java:64

• Draft comment:
  Good use of custom constructors enabling configurable timeouts; this resolves issue #7. Consider documenting that the default timeouts are now 120, 10, 10 seconds respectively.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, suggesting documentation of default timeouts. It doesn't provide a specific code suggestion or highlight a potential issue.

2. src/main/java/nl/dannyj/mistral/services/MistralService.java:120

• Draft comment:
  In method createChatCompletionStream, catching JsonProcessingException per chunk and calling onError terminates the stream. Consider whether a per-chunk error should fail the entire stream or allow partial processing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. src/main/java/nl/dannyj/mistral/serialization/ToolChoiceOptionDeserializer.java:43

• Draft comment:
  The deserializer manually compares string values in a case-insensitive way. This is acceptable, but consider logging or documenting fallback behavior for unexpected strings.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The code's behavior is actually well-structured and clear: it tries strict enum parsing first, then has explicit fallbacks for common cases, and finally throws a descriptive exception. The exception message is already informative. Adding logging here would be unnecessary noise since invalid values should be exceptional. The behavior doesn't need additional documentation since it's straightforward from reading the code. Maybe logging invalid values could help with debugging in production. The fallback behavior isn't explicitly documented in the class's Javadoc. The exception already provides sufficient error information for debugging. The code is self-documenting and follows a clear pattern that's common for enum deserialization. The comment should be deleted as it suggests unnecessary additions to code that is already well-structured and clear.

4. src/main/java/nl/dannyj/mistral/serialization/ContentChunkListDeserializer.java:49

• Draft comment:
  The deserializer returns a singleton TextChunk if a raw string is provided. This is a neat design supporting multi-modal content; ensure that this behavior aligns with all API consumers.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that a specific behavior aligns with all API consumers. This falls under the rule of not asking the author to ensure behavior is intended or to double-check things. Therefore, this comment should be removed.

5. src/main/java/nl/dannyj/mistral/models/completion/ChatCompletionRequest.java:70

• Draft comment:
  Validation annotations (e.g. @NotNull, @SiZe(min = 1)) on the messages field ensure correct usage. Check that the migration instructions in MIGRATION.md cover all these changes.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to check the migration instructions, which is similar to asking them to ensure something is done. This violates the rule against asking the author to double-check things.

6. src/main/java/nl/dannyj/mistral/models/completion/FinishReason.java:24

• Draft comment:
  Using an enum for finishReason enhances type-safety, but ensure consumer code updates comparisons to use enum constants or getReason(), as documented in the migration guide.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to ensure that consumer code updates comparisons to use enum constants or getReason(). This is a general request to ensure something is done, which violates the rule against asking the author to ensure behavior is intended or tested.

7. src/main/java/nl/dannyj/mistral/services/HttpService.java:111

• Draft comment:
  The exception message includes the response string for debugging. Consider potential privacy implications if response bodies contain sensitive data.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a Mistral AI API client library. Error responses from AI APIs typically contain error details that are meant to be seen, not sensitive data. The response body information would be valuable for debugging API issues. The privacy concern seems theoretical rather than practical in this context. I could be wrong about the content of Mistral AI error responses - they might contain sensitive information in some cases. Also, this is a public library that could be used in various contexts. While valid concerns, error messages are crucial for debugging API issues, and Mistral AI's error responses are designed to be seen. The benefit of including error details outweighs the theoretical privacy risk. The comment should be deleted as it would remove valuable debugging information without a concrete privacy benefit in this context.

8. src/main/java/nl/dannyj/mistral/services/MistralService.java:109

• Draft comment:
  CompletableFuture.supplyAsync() is used without an explicit Executor. In high-load scenarios, consider providing a custom executor for better thread management.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. src/main/java/nl/dannyj/mistral/models/completion/message/ChatMessage.java:83

• Draft comment:
  The getTextContent() method iterates through the content list which is efficient for small messages. For messages with large amounts of multi-modal content, consider potential performance implications, though it's unlikely to be a bottleneck.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment is informative and suggests considering performance implications, but it doesn't provide a specific action or suggestion for improvement. It doesn't ask for confirmation or suggest a specific change, so it doesn't align with the rules for good comments.

10. src/main/java/nl/dannyj/mistral/models/completion/tool/ToolChoiceOption.java:27

• Draft comment:
  The sealed interface ToolChoiceOption is a neat use of Java 17 sealed types. Ensure client code on older Java versions is aware of the compatibility requirements.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to ensure that client code is aware of compatibility requirements, which is a form of asking them to double-check or verify something. This violates the rule against asking the author to ensure or verify things.

11. src/main/java/nl/dannyj/mistral/MistralClient.java:66

• Draft comment:
  Good improvement: Default HTTP client now uses buildHttpClient(120,10,10) instead of a fixed 20-second read timeout. This addresses issue #7 by allowing longer requests and enabling customization via additional constructors.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, explaining a change in the HTTP client configuration. It doesn't suggest any action or raise any concerns about the change. According to the rules, purely informative comments should be removed.

12. src/main/java/nl/dannyj/mistral/MistralClient.java:129

• Draft comment:
  The additional constructors allowing custom read, connect, and write timeout configuration provide flexibility. Ensure your documentation highlights these new API methods for configuring timeouts.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to ensure that documentation is updated, which is similar to asking them to confirm or ensure something. It doesn't provide a specific code suggestion or point out a specific issue with the code itself.

13. src/main/java/nl/dannyj/mistral/models/completion/ChatCompletionRequest.java:142

• Draft comment:
  The migration of the messages field from List to List is clear. The updated Javadoc explains how to build the conversation correctly – very useful for developers migrating their code.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, praising the clarity of the migration and the usefulness of the updated Javadoc. It doesn't provide any actionable feedback or suggestions for improvement.

14. src/main/java/nl/dannyj/mistral/models/completion/Choice.java:47

• Draft comment:
  Changing finishReason from a String to the FinishReason enum is a breaking change; the migration guide covers this well. Consider adding unit tests to verify proper enum handling.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment correctly identifies a breaking change in the API. However, suggesting unit tests is not very actionable - the author likely knows they need to test their changes. The breaking change itself is probably intentional since it's part of a larger refactoring (Message -> AssistantMessage). We should trust the author's judgment on API changes. The comment identifies a real technical concern about API compatibility. Breaking changes could affect downstream users. While API compatibility is important, the change appears intentional as part of a larger refactoring. The suggestion to add tests is too vague to be actionable. Delete the comment. While it identifies a real breaking change, the change appears intentional and the testing suggestion is not specific enough to be useful.

15. src/main/java/nl/dannyj/mistral/serialization/ContentChunkListDeserializer.java:49

• Draft comment:
  The custom deserializer for ContentChunk handles both string and complex JSON correctly. Double-check that repeatedly calling responseBody.string() (as seen in HttpService) does not cause issues with stream consumption.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to double-check something, which violates the rule against asking for confirmation or verification. It does not provide a specific suggestion or point out a clear issue.

16. src/main/java/nl/dannyj/mistral/serialization/ToolChoiceOptionDeserializer.java:43

• Draft comment:
  The logic in ToolChoiceOptionDeserializer for converting string values looks robust. Converting the token text to uppercase before calling valueOf is a pragmatic workaround.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the existing logic without offering any constructive criticism or questions.

17. src/main/java/nl/dannyj/mistral/services/HttpService.java:109

• Draft comment:
  In executeRequest(), the response body is read in the error branch, which might consume the stream. Consider caching the body string if you need it later.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is technically correct - calling response.body().string() does consume the stream. However, this is in the error branch where we're throwing an exception immediately after. The response body isn't needed again since we're terminating execution. The successful path uses a separate try-with-resources block to handle the response body properly. Maybe there could be some error handling or logging code that needs the response body later? Or perhaps future maintenance could be impacted by this consumed stream? Since this is in an exception throwing branch, the consumed stream won't matter - the exception will terminate execution immediately. Any error handling would happen in the catch block which already has the error message including the body. The comment should be deleted because while technically correct, it raises a non-issue since the stream consumption happens right before throwing an exception.

18. src/main/java/nl/dannyj/mistral/services/MistralService.java:87

• Draft comment:
  The check ensuring the first message role is either 'user' or 'system' is important—it's a good safeguard against misuse of the API.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

19. src/main/java/nl/dannyj/mistral/services/MistralService.java:265

• Draft comment:
  The streaming implementation in handleResponseBody() correctly processes chunk lines and checks for the '[DONE]' signal.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

20. src/main/java/nl/dannyj/mistral/models/completion/message/ChatMessage.java:85

• Draft comment:
  The convenience method getTextContent() in ChatMessage elegantly aggregates text from all TextChunks. This abstraction is useful for consumers.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative, praising the implementation without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.

21. CODE_OF_CONDUCT.md:109

• Draft comment:
  Typo: Removed the extra space between 'behavior,' and 'harassment'. Please ensure that spacing is consistent for better readability.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

22. MIGRATION.md:129

• Draft comment:
  There's an awkward line break in the code snippet for extracting text content from responseMsg. The expression if(responseMsg. getContent() !=null){ appears to be split improperly. Please merge it into a single line (or format it consistently) to improve readability.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a new file being added (MIGRATION.md). The formatting issues do make the code examples harder to read and understand. However, this is just a documentation file, and the formatting issues don't affect actual code. The comment is about a purely cosmetic issue. Per the rules, we should not make purely informative comments that don't require clear code changes. The formatting issues could make it harder for developers to copy-paste and use these examples. Bad formatting in documentation could lead to confusion. While the formatting could be better, this is still just a documentation issue. The rules clearly state we should not make comments unless there is clearly a code change required. Delete this comment as it's about documentation formatting rather than actual code changes that need to be made.

23. MIGRATION.md:264

• Draft comment:
  In the code snippet demonstrating handling of finishReason, there's an unintended line break splitting if("stop". equals(reason)){. Please combine the string literal so it reads as if ("stop".equals(reason)) { on one line for clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a documentation file showing migration examples. The split line is in a "Before" code example showing old code that needs to be migrated away from. The formatting doesn't impact understanding since it's just an example of old code that needs to be changed anyway. The comment is purely about formatting in documentation. The split line could potentially make the example harder to copy-paste. Documentation quality does matter for developer experience. Since this is in a "Before" example showing code that needs to be migrated away from, developers shouldn't be copy-pasting this code anyway - they should use the "After" example instead. Delete the comment. Formatting nitpicks on documentation examples, especially ones showing old code to migrate away from, don't provide enough value.

24. MIGRATION.md:236

• Draft comment:
  The explanation for getting the underlying string value from FinishReason contains a typographical issue: it reads e.g ., "stop","length" which appears inconsistent. Consider changing it to something like e.g., "stop", "length" for clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a new file being added, so it is part of the changes. However, the comment is purely about formatting/readability and doesn't impact the technical accuracy or functionality of the documentation. Our rules emphasize focusing on logic issues and avoiding purely informative or obvious comments. The inconsistent formatting could potentially make the documentation slightly harder to read. The suggestion is clear and actionable. While the suggestion is valid, this kind of minor formatting issue is not significant enough to warrant a PR comment given our rules about focusing on substantive issues. Delete this comment as it addresses a trivial formatting issue rather than a substantive problem with the code or documentation content.

25. README.md:122

• Draft comment:
  Typo: In the code comment and string literal on line 122, 'Write a java program...' should be updated to 'Write a Java program...' to maintain proper capitalization.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a very minor capitalization issue. The rules specifically say not to make comments that are obvious or unimportant. Capitalization of "Java" vs "java" in a code comment or string literal that will only be seen by the user of the program is extremely minor and doesn't affect functionality at all. Could inconsistent capitalization of "Java" make the documentation look unprofessional since this is a public API client library? While professional documentation is important, this string is just an example prompt being passed to an AI model - it's not part of the actual documentation. The capitalization here has no meaningful impact. This comment should be deleted as it violates the rule about not making obvious or unimportant comments. The capitalization issue is too minor to warrant a PR comment.

26. src/main/java/nl/dannyj/mistral/models/completion/tool/Function.java:60

• Draft comment:
  Typo: In the Javadoc comment for the 'parameters' field, change "The parameters the functions accepts" to "The parameters the function accepts" to correct the grammatical error.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is a real typo, it's a very minor documentation issue. The meaning is still completely clear despite the typo. The rules state we should not make purely informative comments or comments that are obvious/unimportant. Documentation typos generally fall into this category unless they significantly impact understanding. The typo could be seen as unprofessional in public API documentation. Poor documentation can impact developer experience. While documentation quality matters, this particular typo is so minor that fixing it provides negligible value. The meaning is still perfectly clear. This comment should be deleted as it addresses a trivial documentation issue that doesn't impact code functionality or understanding.

27. src/main/java/nl/dannyj/mistral/services/HttpService.java:89

• Draft comment:
  Consider renaming the parameter 'callBack' to 'callback' in the streamPost method for consistency.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

28. src/main/java/nl/dannyj/mistral/services/HttpService.java:104

• Draft comment:
  In the Javadoc for executeRequest, consider changing 'objectmapper' to 'object mapper' for clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_x3K3QYLwi8m0CmvC

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed dda7ff8 in 1 minute and 45 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/main/java/nl/dannyj/mistral/models/completion/ChatCompletionRequest.java:67

• Draft comment:
  The @param description for temperature now allows values between 0.0 and 1.5, which matches the validation annotations. However, the PR description mentions resolving client timeout issues, but there is no change related to timeout configuration here.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. src/main/java/nl/dannyj/mistral/models/completion/ChatCompletionRequest.java:67

• Draft comment:
  The temperature parameter documentation has been updated to allow values up to 1.5, which matches the @DecimalMax annotation. Verify that this new upper bound is intentional per Mistral API v0.0.2.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

3. src/main/java/nl/dannyj/mistral/models/completion/ChatCompletionRequest.java:50

• Draft comment:
  The PR description mentions resolving the client timeout issue (issue #7), but no changes for configuring client timeout are evident in this diff. Ensure that the timeout parameter is made configurable as intended.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_wMgeGPrhpJIQOdve

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
4 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed ebc2d74 in 2 minutes and 14 seconds. Click for details.

• Reviewed 71 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. .github/workflows/build.yml:1

• Draft comment:
  The workflow is now labeled 'SonarQube' (L1) while the sonar.host.url in build.gradle (L48) still points to 'sonarcloud.io'. Confirm if the naming change is intentional and if all configuration values should be consistent.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment asks for confirmation and explanation ("Confirm if..."), which violates our rules. It's speculative about whether there might be an issue. It also requires understanding of the broader system configuration which goes beyond the scope of this single file review. The workflow name is just a label and doesn't necessarily need to match the service URL. The comment might be pointing out a genuine configuration issue that could cause problems. The name change could indicate an intended migration from SonarCloud to SonarQube. While there might be a real issue, the comment violates multiple rules: it asks for confirmation, is speculative, and requires cross-file context. If there was a real configuration problem, it would likely be caught by the build. The comment should be deleted as it violates multiple review rules and any real configuration issues would be caught during build/deployment.

2. build.gradle:4

• Draft comment:
  The SonarQube plugin version was downgraded from 6.1.0.5360 to 6.0.1.5171. Please verify that this downgrade was intended and compatible with the current configuration.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify their intention regarding a dependency change, which violates the rule against asking for confirmation of intention. It also involves a dependency version change, which is not recognized as a valid comment topic according to the rules.

3. .github/workflows/build.yml:1

• Draft comment:
  The workflow name was changed from 'SonarCloud' to 'SonarQube'. Confirm that the sonar host configuration is updated accordingly (build.gradle still points to sonarcloud.io).
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment asks to verify configuration in another file (build.gradle). This violates two rules: 1) Don't ask authors to verify/confirm things, and 2) Ignore cross-file issues. While the concern might be valid, it requires context from another file that we can't see. The comment might be pointing out a legitimate configuration mismatch that could cause build failures. Maybe we should keep it to prevent potential issues? Even if there is a potential issue, the rules explicitly state to ignore cross-file issues and not to make speculative comments. If there's a real configuration issue, it would be caught by the build. Delete the comment as it violates multiple rules: it asks for verification, requires cross-file context, and any real issues would be caught by the build system.

4. .github/workflows/build.yml:35

• Draft comment:
  Removal of the GITHUB_TOKEN environment variable in the build step may affect PR detail retrieval. Verify that it’s no longer needed for your analysis.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to verify the necessity of the GITHUB_TOKEN environment variable, which violates the rule against asking for confirmation or verification of intention. It does not provide a specific suggestion or point out a clear issue.

5. build.gradle:4

• Draft comment:
  The SonarQube plugin version was downgraded from 6.1.0.5360 to 6.0.1.5171. Please confirm this change is intentional and compatible with your setup.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is about a dependency version change, which is not allowed according to the rules. The comment asks for confirmation about the change, which is also not allowed. Therefore, this comment should be removed.

6. .github/workflows/build.yml:37

• Draft comment:
  The file is missing a trailing newline at the end. Please add a newline to adhere to best practices.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_l5IHKyaImpxPIkWz

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}