-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Bring back QA changes 0.6.9 #6320
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* fix: check for env value before setting * Use empty instead of none
The local build script for Linux was failing due to a bundling error. This commit updates the `build:tauri:linux` script in `package.json` to be consistent with the CI build pipeline, which resolves the issue. The updated script now includes: - **`NO_STRIP=1`**: This environment variable prevents the `linuxdeploy` utility from stripping debugging symbols, which was a potential cause of the bundling failure. - **`--verbose`**: This flag provides more detailed output during the build, which can be useful for debugging similar issues in the future.
…d-model fix: compatibility imported model
fix: update copy offload_mmproj setting desc
…model fix: handle manual toggle vision for remote model
…itor fix: system monitor window permission
fix: update ui info
chore: update credit description
chore: bundle license to app
fix: handle paste image on linux
feat: add regression checklist
fix: mcp cleanup dropodown tool availabel and sort list
fix: handle conditional UI regenerate resp
fix: code generation more than 300 line
…-gated fix: handle checking model gated
qnixsynapse
approved these changes
Aug 28, 2025
Minh141120
approved these changes
Aug 28, 2025
urmauur
approved these changes
Aug 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 7a65747 in 3 minutes and 20 seconds. Click for details.
- Reviewed
1718lines of code in28files - Skipped
0files when reviewing. - Skipped posting
23draft 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. extensions/llamacpp-extension/settings.json:7
- Draft comment:
Changed default value from "none" to empty string. Confirm that an empty string is the intended default for environmental variables. - 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.
2. extensions/llamacpp-extension/src/index.ts:1372
- Draft comment:
Added null-check for this.llamacpp_env before calling parseEnvFromString. Verify that this logic consistently prevents errors when the env string is undefined. - 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. package.json:25
- Draft comment:
The linux build script now uses NO_STRIP=1 and adds the --verbose flag. Confirm that these changes are intended for debugging and don't affect the release build performance. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm their intention regarding the use of NO_STRIP=1 and the --verbose flag in the build script. This falls under the rule of not asking the author to confirm their intention or ensure behavior is intended. Therefore, this comment should be removed.
4. web-app/src/containers/ChatInput.tsx:377
- Draft comment:
The handlePaste function now includes modern Clipboard API fallback (navigator.clipboard.read). Ensure that errors in clipboard.read are properly handled to prevent unhandled rejections. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. web-app/src/containers/ModelSupportStatus.tsx:68
- Draft comment:
The YAML fallback logic using invoke('read_yaml') is a good addition. Consider caching its result per model to avoid redundant file reads when repeatedly checking support. - 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 YAML reading only happens when the model.gguf file doesn't exist and we fall back to model.yml. This is likely an edge case for imported models. The function is already memoized and only runs when dependencies change. Adding caching would add complexity for minimal benefit since the reads only happen on model changes, not continuously. I could be underestimating how frequently this code path is hit. If models are frequently switched or if the support check is called from other places, caching could be more valuable. Even if the code path is hit more frequently, the complexity of adding caching may not be worth it given this appears to be a UI status indicator that isn't performance critical. The comment suggests a premature optimization that would add complexity for minimal benefit, given the YAML reading only happens on model changes and is already memoized.
6. web-app/src/containers/RenderMarkdown.tsx:73
- Draft comment:
Removal of the virtualized renderer simplifies code. Verify that performance for very large code blocks remains acceptable without virtualization. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment asks the PR author to verify performance, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
7. web-app/src/hooks/useMCPServers.ts:70
- Draft comment:
The newly added renameServer action preserves insertion order. Ensure tests cover edge cases for renaming. - 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 has two parts: 1) stating that insertion order is preserved (which is just stating what the code already clearly shows), and 2) suggesting testing (which violates the rule about not asking authors to verify/test things). The comment doesn't point out any actual issues that need fixing. The code's order preservation might be an important feature that deserves documentation. Maybe suggesting tests is valuable for complex functionality like this. While order preservation might be important, it's already clearly documented in the code comments. Suggesting tests violates our rules and doesn't provide actionable feedback. The comment should be deleted as it's purely informative and contains a testing suggestion that violates our rules.
8. web-app/src/services/models.ts:594
- Draft comment:
In isModelSupported, errors now return 'GREY' for engine failures. This is consistent with tests, but ensure this fallback meets expected UX. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =40%<= threshold50%The comment is pointing out a change in behavior where errors return 'GREY' for engine failures. It asks to ensure that this fallback meets expected UX, which is a request for confirmation. However, it also provides a specific observation about the change being consistent with tests, which could be useful. The comment is borderline, but it leans towards being informative and asking for confirmation.
9. autoqa/checklist.md:9
- Draft comment:
Typo: "Ensure there are 2 model downloaded" should likely be "Ensure there are 2 models downloaded". - 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% While this is technically a grammatical error, the review rules state not to make purely informative comments and to only comment if there's clearly a code change required. This is a checklist file, and the meaning is perfectly clear even with the minor grammatical error. The comment doesn't improve the functionality or quality of the code. The grammar is technically incorrect, and fixing it would make the document more professional. Documentation quality could be considered important. The rules explicitly state not to make purely informative comments. The meaning is clear despite the minor grammatical error, and this isn't a user-facing document - it's an internal checklist. Delete this comment as it's purely informative and doesn't affect functionality or code quality.
10. autoqa/checklist.md:10
- Draft comment:
Typo: "Ensure there are 2 import on local provider (llama.cpp)" should probably be "Ensure there are 2 imports on local provider (llama.cpp)". - 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 very minor grammatical fix in a checklist document. The rules state not to make purely informative comments or comments about obvious issues. While technically correct, this small grammatical error doesn't impact the understanding or functionality of the checklist. The grammar correction is technically accurate. Could improving documentation clarity be considered important enough to keep? While clear documentation is valuable, this minor grammatical issue doesn't significantly impact clarity or understanding. The rules specifically say not to make obvious or unimportant comments. Delete this comment as it's too minor and doesn't materially improve the document or require action from the PR author.
11. autoqa/checklist.md:22
- Draft comment:
Typo: "Previously used model and assistants is shown correctly" should use plural verb, e.g., "are shown correctly". - 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 new checklist file being added. While the grammar correction is technically correct, this is a checklist document meant for internal testing use. Minor grammatical issues don't impact the functionality or understanding of the checklist. The comment violates the rule about not making purely informative comments that don't require code changes. The grammar error could theoretically make the checklist item slightly less clear or professional. Some might argue any documentation should be grammatically correct. The meaning is perfectly clear despite the minor grammar issue. This is an internal checklist, not user-facing documentation. Nitpicking grammar in checklists adds noise without value. Delete this comment as it's a purely informative grammar correction that doesn't materially impact the usefulness or clarity of this internal checklist document.
12. autoqa/checklist.md:58
- Draft comment:
Typo: "un-standard character" – consider using "non-standard character". - 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 "non-standard" is indeed more correct than "un-standard", this is a minor grammatical issue in a checklist file. The meaning is still clear with the current wording. The comment doesn't point out any functional issues or code quality problems. It's more of a nitpick on documentation wording. The suggestion is technically correct - "non-standard" is the proper form. Documentation quality and clarity does matter. However, the current wording is still understandable and this is an internal checklist file, not user-facing documentation. The comment adds noise without significant value. Delete this comment as it's too minor of a documentation wording issue to warrant a PR comment.
13. autoqa/checklist.md:107
- Draft comment:
Typo: "Modesl list view" should be "Models list view". - 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 technically a typo, the review rules state not to make purely informative comments or comments about obvious issues. This is a very minor typo that doesn't affect functionality or understanding. The file is a checklist for QA testing, not user-facing documentation or code. The typo could theoretically cause confusion when someone is searching for this section in the document. Also, since this is a QA checklist, accuracy might be more important than in other contexts. The potential for confusion is very low - the meaning is completely clear from context. This is exactly the kind of minor, obvious issue that the review rules tell us to avoid commenting on. Delete this comment. It points out a real but extremely minor issue that doesn't warrant a formal review comment.
14. autoqa/checklist.md:110
- Draft comment:
Typo: "Ollama set-up as a custom provider work with Jan" should be corrected to "Ollama set-up as a custom provider works with Jan". - 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% The rules state not to make purely informative comments and to only comment if there's clearly a code change required. While this is a grammatical error, fixing it doesn't impact functionality. The file is a checklist document, not code, and small grammar issues don't affect its usability. The grammar error could make the checklist look unprofessional. Multiple small errors can add up to make documentation harder to read. While documentation quality matters, this is a minor grammatical error that doesn't impact understanding. The rules explicitly say not to make purely informative comments. Delete this comment as it's a minor grammatical suggestion that doesn't affect functionality or understanding, violating the rule about not making purely informative comments.
15. autoqa/checklist.md:112
- Draft comment:
Typo: "create a new custom providers" – consider using either "a new custom provider" or "new custom providers". - 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 the grammar error is real, this is a checklist file meant for internal testing. The meaning is perfectly clear despite the minor grammatical error. The comment doesn't point out any functional issues or testing gaps. We should focus on substantive issues rather than minor grammar fixes in testing documentation. The grammar error could potentially cause confusion about whether the test should verify single or multiple provider creation. Maybe this ambiguity matters for testing coverage? Looking at the surrounding context and other checklist items, it's clear this is testing the basic functionality of creating a custom provider. The slight grammar issue doesn't impact the testing instructions or coverage. Delete this comment. While technically correct, grammar fixes in internal testing documentation are not important enough to warrant review comments unless they cause actual confusion about testing requirements.
16. autoqa/checklist.md:158
- Draft comment:
Typo: Extra trailing character in "pop-up.a" – it should be "pop-up". - 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% While this is technically a change in the new file since this is a new file being added, the rules state not to make purely informative comments or comments about obvious/unimportant things. A single character typo in a markdown checklist file is very minor and doesn't affect functionality. The typo could theoretically cause confusion for users reading the checklist, so maybe it's worth fixing? The typo is so minor and in a checklist file that it's extremely unlikely to cause any real confusion. The meaning is still completely clear. This comment should be deleted as it points out an extremely minor typo that doesn't impact functionality or clarity.
17. autoqa/checklist.md:159
- Draft comment:
Typo: "MCp" should be "MCP". - 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% While this is a real typo, it's an extremely minor issue in a checklist file. The meaning is still completely clear. The typo doesn't affect functionality or cause any confusion. This feels like the kind of nitpicky comment that just adds noise to the PR review process. The typo is real and technically incorrect. Consistency in technical terms can be important for documentation. While consistency is good, this is an internal checklist file and the meaning is completely clear. The benefit of fixing this tiny typo doesn't outweigh the cost of the noise added to the PR review. This comment should be removed as it's too minor and doesn't provide meaningful value to the PR review process.
18. autoqa/checklist.md:212
- Draft comment:
Typo: "The token speed appear ... and the final value is show" should be "The token speed appears ... and the final value is shown". - 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 checklist file for QA testing. While the grammar correction is technically accurate, the rules state not to make purely informative comments or comments about obvious issues. Grammar fixes in documentation, unless they significantly impact meaning, are generally considered minor and obvious. The grammar error could potentially cause confusion for non-native English speakers. Documentation quality is important for international users. While documentation quality matters, this particular grammar issue doesn't meaningfully impact comprehension. The meaning is clear despite the minor grammatical errors. Delete this comment as it's a minor grammatical suggestion that doesn't materially impact the usefulness or clarity of the checklist.
19. autoqa/checklist.md:213
- Draft comment:
Typo: "after each words" should be "after each word". - 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% While the grammar correction is technically correct, the review rules state not to make purely informative comments and to only comment if there's clearly a code change required. This is a documentation file and the meaning is clear despite the minor grammatical error. The comment doesn't affect functionality or code quality. The grammar error could make the documentation look unprofessional. Documentation quality is important for user experience. However, the review rules explicitly state not to make purely informative comments. The meaning is still clear despite the minor error. Delete this comment as it's purely informative and doesn't affect functionality or code quality.
20. autoqa/checklist.md:223
- Draft comment:
Typo: "will be come" should be "will become". - 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 the comment is technically correct about the grammar, this is a checklist file for internal QA testing. Small grammatical issues don't impact the functionality or clarity of the checklist. The meaning is still clear even with the minor grammatical error. The grammar error could make the document look less professional. Multiple small errors can add up to make documentation feel sloppy. This is an internal QA checklist, not customer-facing documentation. The focus should be on technical accuracy and clarity of the test steps rather than perfect grammar. While technically correct, this grammar fix is too minor to warrant a PR comment on an internal checklist file.
21. autoqa/checklist.md:232
- Draft comment:
Typo: "All Assistant deleted" should be "All Assistants deleted". - 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% While this is technically a grammatical error, it's in a checklist file and the meaning is perfectly clear. The comment is about something that was changed (this is a new file). However, our rules say not to make purely informative comments or comments about obvious/unimportant issues. This is a very minor grammatical issue that doesn't affect functionality or understanding. The comment is technically correct - it is a real grammatical error. And since this is a new file, it's fair game for commenting. However, our rules explicitly state not to make comments that are obvious or unimportant. This minor grammatical fix doesn't meaningfully improve the code or documentation. We should delete this comment as it violates our rule about not making unimportant comments. The minor grammatical error doesn't impact understanding.
22. autoqa/checklist.md:246
- Draft comment:
Typo: "left over folder" should be "leftover folders". - 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 rules state not to make purely informative comments and to avoid obvious or unimportant changes. While this is technically a grammatical improvement, it's a very minor one that doesn't affect understanding or functionality. The checklist is an internal document, not user-facing documentation where perfect grammar would be more important. The change would make the text more grammatically correct. Perhaps poor grammar in documentation could reflect badly on the project's quality? The grammatical issue is so minor that it doesn't impact readability or understanding. The rules explicitly say to avoid obvious or unimportant comments. This comment should be deleted as it's too minor and doesn't affect functionality or clarity.
23. web-app/src/containers/dialogs/EditModel.tsx:207
- Draft comment:
There appears to be an extraneous comment terminator (*/) appended after the closingdivtag. This may be a typographical error—please verify if it was intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_CbR8HqSHFj7hye8Q
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe Your Changes
Fixes Issues
Self Checklist
Important
Reintroduces QA changes for version 0.6.9, enhancing model handling, download management, and UI components, with updates to model support checks and MCP server management.
isModelSupported()to return 'GREY' on errors inmodels.tsandmodels.test.ts.checkMmprojExistsAndUpdateOffloadMMprojSetting()andcheckMmprojExists()inmodels.tsfor better model management.ModelInfoHoverCardandModelSupportStatuscomponents for model compatibility display.DropdownModelProviderandDownloadManagementfor improved user interaction.RenderMarkdownandThreadContentfor better content rendering.useMCPServers.tsandmcp-servers.tsx.$providerName.tsxanduseModelProvider.ts.LICENSEfile to reflect new copyright information.This description was created by
for 7a65747. You can customize this summary. It will automatically update as commits are pushed.