Skip to content

Conversation

@louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Jul 23, 2025

Describe Your Changes

This PR is to address the issue where:

  1. llama.cpp version list shows blank options
  2. App shows legacy llama.cpp provider

#5444

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Fixes blank options in llama.cpp version list and filters out legacy llama.cpp provider in extension.ts and useModelProvider.ts.

  • Behavior:
    • Fixes blank options in llama.cpp version list by updating registerSettings() in extension.ts to populate options from old settings if missing.
    • Filters out legacy llama.cpp provider in setProviders() in useModelProvider.ts to prevent it from appearing in the list.
  • Misc:
    • Adds comments in useModelProvider.ts to indicate the temporary nature of filtering out the legacy provider.

This description was created by Ellipsis for fb6babb. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed everything up to fb6babb in 1 minute and 57 seconds. Click for details.
  • Reviewed 50 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 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. core/src/browser/extension.ts:131
  • Draft comment:
    When assigning controllerProps.options, the fallback to old settings on an empty array may override an intentional empty state. Confirm that an empty array is not a valid choice.
  • 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 raises a valid point about data preservation logic - an empty array could be intentionally set. However, this is a speculative concern ("may override") and asks for confirmation rather than pointing out a definite issue. The comment starts with verification language which violates our rules. We don't have enough context to know if empty options are valid or not. The code could have legitimate reasons for treating empty arrays as "unset" values that should fall back to previous settings. Without more context about the settings system, we can't be certain this is a bug. While the concern about data preservation is valid, the comment's speculative nature and request for confirmation rather than pointing out a clear issue makes it less useful. Delete this comment as it is speculative and asks for confirmation rather than identifying a clear issue that needs fixing.
2. web-app/src/hooks/useModelProvider.ts:40
  • Draft comment:
    Legacy provider filtering removes providers with key 'llama.cpp', but note that the default selectedProvider is 'llamacpp'. Verify that the naming is consistent to avoid selection issues post-migration.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_EnBoASiSKqIRHzpw

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 23, 2025

Barecheck - Code coverage report

Total: 36.95%

Your code coverage diff: 0.00% ▴

Uncovered files and lines
FileLines
core/src/browser/extension.ts93-94, 101-104, 113-115, 124-125, 127-136, 139-140, 174, 176-185
web-app/src/hooks/useModelProvider.ts31-36, 38-39, 42-52, 54-56, 58-107, 109-120, 130-132, 134, 136-138, 141-144, 146-147, 149, 151-153, 155-168, 170-173, 175-180

@louis-jan louis-jan force-pushed the fix/llamacpp-backend-list-blank-sometime branch from fb6babb to d0ee2ac Compare July 23, 2025 12:19
@louis-jan louis-jan merged commit d6ad797 into release/v0.6.6 Jul 23, 2025
17 of 20 checks passed
@louis-jan louis-jan deleted the fix/llamacpp-backend-list-blank-sometime branch July 23, 2025 13:04
@github-project-automation github-project-automation bot moved this to QA in Jan Jul 23, 2025
@github-actions github-actions bot added this to the v0.6.6 milestone Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants