Skip to content

Conversation

@louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Aug 19, 2025

Describe Your Changes

This PR adds model tool use capability detection, eliminating the need for manual model capabilities edits.

How it works:

  • It reads chat_template from model GGUF, detects tools, and sets model tool support automatically.

It also addresses an issue where the model download race condition occurs, resulting in successful download but not display in the list.

CleanShot.2025-08-19.at.10.03.21.mp4

Tool indicator in hub
CleanShot 2025-08-19 at 10 40 38@2x

Fixes Issues

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

This PR adds automatic detection of model tool use capabilities from GGUF metadata and fixes a model download race condition.

  • Behavior:
    • Adds isToolSupported() to AIEngine and llamacpp_extension to auto-detect tool support from GGUF metadata.
    • Emits AppEvent.onModelImported in llamacpp_extension after model import.
    • Removes manual model capability edits in EditModel.tsx.
  • Events:
    • Adds onModelImported to AppEvent in index.ts.
    • Registers onModelImported event in DataProvider.tsx to refresh providers.
  • UI:
    • Removes DialogEditModel from $providerName.tsx.
    • Displays tool support icon in hub/index.tsx if model supports tools.
  • Misc:
    • Fixes model download race condition in DownloadManagement.tsx by removing redundant provider refresh.
    • Updates tests in providers.test.ts to mock isToolSupported().
    • Removes DefaultToolUseSupportedModels enum from models.ts.

This description was created by Ellipsis for d7cf258. 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.

Caution

Changes requested ❌

Reviewed everything up to 0dc0a9a in 4 minutes and 7 seconds. Click for details.
  • Reviewed 804 lines of code in 11 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. core/src/browser/extensions/engines/AIEngine.ts:275
  • Draft comment:
    Good addition of an abstract isToolSupported method to enforce tool capability detection in all engines.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. extensions/llamacpp-extension/src/index.ts:1625
  • Draft comment:
    Consider explicitly converting the result of the GGUF metadata check to a boolean. Using 'return Boolean((await readGgufMetadata(modelPath)).metadata?.["tokenizer.chat_template"]?.includes("tools"))' ensures that undefined is coerced to false.
  • 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 the suggestion would make the code slightly more explicit, the current code is already type-safe due to TypeScript's type system and optional chaining. The function is declared to return Promise so TypeScript will ensure the return type is correct. The current code is idiomatic TypeScript and the suggested change would add verbosity without meaningful benefit. The comment points out a valid way to make the code more explicit, but I may be underestimating the value of being extra clear about boolean coercion. The current code already handles undefined correctly through optional chaining and the TypeScript return type ensures type safety. Making it more explicit would just add noise. Delete the comment since the current code is already type-safe and idiomatic TypeScript. The suggested change would add unnecessary verbosity.
3. extensions/llamacpp-extension/src/index.ts:1099
  • Draft comment:
    Typographical error: The comment uses "prepand" but it should be "prepend" to correctly indicate adding a prefix.
  • 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.
4. web-app/src/services/models.ts:318
  • Draft comment:
    Typo in documentation: "Check if model support tool use capability" should likely be "Check if model supports tool use capability."
  • 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, it's a very minor documentation issue. Documentation comments are for developers, and the meaning is perfectly clear despite the minor grammatical error. This kind of nitpicky comment about documentation doesn't require a code change and doesn't impact functionality. The comment is factually accurate - there is indeed a grammatical error. And clear documentation is important for code maintainability. However, per the rules, we should not make purely informative comments that don't require clear code changes. Minor documentation grammar fixes fall into this category. This comment should be removed as it's too minor and doesn't highlight an actual code issue that needs fixing.
5. web-app/src/services/models.ts:319
  • Draft comment:
    Typo in documentation: "Returned by backend engine" might be clearer as "Returned by the backend engine."
  • 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% Documentation comments are important, but this is an extremely minor grammatical suggestion that doesn't affect understanding. The original "Returned by backend engine" is perfectly clear. This kind of nitpicky grammar suggestion creates noise without adding value. We should focus on substantive issues. Documentation clarity is important for maintainability. Maybe improving grammar helps non-native English speakers? While documentation clarity matters, this change is so minor it doesn't meaningfully improve understanding. The original is already clear enough. Delete this comment as it suggests an extremely minor grammatical change that doesn't meaningfully improve the documentation's clarity.
6. web-app/src/types/models.ts:16
  • Draft comment:
    There appears to be no newline at the end of the file. Please add a newline at the end to adhere to common style guidelines.
  • 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 having a newline at the end of file is a common convention, this is something that should be handled by editor settings or linting rules, not PR comments. It's a minor stylistic issue that doesn't affect functionality. Additionally, this isn't related to the actual changes being made in the PR (removing an enum). The missing newline could cause issues with some tools or make git diffs less clean. It is a real issue that should be fixed according to most style guides. While it's a valid style issue, this is exactly the kind of thing that should be caught by automated tooling rather than manual review comments. It's too minor for a PR comment and not related to the actual changes. Delete this comment. It's a minor style issue that should be handled by linting rules, and it's not related to the actual changes in the PR.

Workflow ID: wflow_S5r5P0TR6UAwFICm

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 Aug 19, 2025

Barecheck - Code coverage report

Total: 37.62%

Your code coverage diff: 0.29% ▴

Uncovered files and lines
FileLines
web-app/src/containers/DownloadManegement.tsx1, 6-15, 17-28, 30-35, 37-44, 46-47, 51-61, 63-72, 74-82, 84, 86-92, 95-103, 105-106, 108-113, 115-121, 124-129, 131-132, 134-140, 142-154, 156-169, 171-178, 180-193, 195-200, 203-205, 207-212, 215-230, 232-235, 237-258, 260-270, 272, 274-279, 281-291, 293-301, 303-304, 306-309, 311-353, 355, 357
web-app/src/providers/DataProvider.tsx40-42, 45, 56-58, 70-71, 76, 86-87, 89, 92, 94-100
web-app/src/routes/hub/index.tsx2-7, 15-20, 26-29, 35, 41-47, 55, 57-62, 64-66, 68-75, 77-79, 81, 83-99, 101-102, 104-109, 112-119, 121-123, 126, 128-133, 135-136, 138-139, 141-146, 148-156, 158-169, 172-176, 178-180, 182-185, 187-189, 191-195, 197-198, 200-223, 225-226, 228-238, 240, 242-245, 247-261, 263-264, 266-277, 279-280, 282, 284-300, 302, 304-307, 309-314, 316-322, 324-328, 330-331, 333-338, 340-341, 343, 345-355, 357-358, 361-370, 372-373, 375-391, 393-397, 400-401, 404-405, 407-424, 427, 429-434, 436-437, 439-449, 451-465, 467, 469-502, 504-507, 509-534, 536-545, 547-563, 565-579, 582-588, 591-600, 602-607, 610-620, 622-626, 628-665, 667-675, 677-681, 683-687, 689-723, 725-731, 733-737, 739-741, 743, 745-749, 751-757, 760-762, 764, 766-781, 783-787, 789-790, 792, 794-795, 797-801, 803-807, 809
web-app/src/routes/settings/providers/$providerName.tsx2-8, 15, 21-42, 45-47, 49-53, 55-85, 88-96, 98, 100, 103-105, 107-108, 111-112, 114, 116-121, 124-125, 127-130, 135-136, 138-141, 143-149, 151-153, 156-162, 165-168, 170, 172-176, 178-202, 204, 206-220, 222-224, 226-236, 238-275, 277-283, 286-287, 289-296, 298-308, 312-314, 317, 320-322, 324-335, 338-342, 344-347, 350-351, 354, 356-360, 362-369, 371-374, 376, 379-388, 390-395, 397-407, 409-411, 413-425, 427, 429-430, 432, 434-435, 438-455, 457-462, 464-467, 469-475, 477-478, 480-495, 497-500, 502-510, 512-522, 524-533, 535-536, 539-549, 551-554, 556-562, 564-573, 575-587, 589-590, 592-597, 599-605, 607, 609, 611, 613, 615, 617, 619-626, 628-630, 632-638, 640
web-app/src/services/models.ts130-132, 307, 325-326, 328-329
web-app/src/services/providers.ts30, 55, 75, 77, 177-178

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 d7cf258 in 1 minute and 18 seconds. Click for details.
  • Reviewed 58 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. web-app/src/routes/hub/index.tsx:658
  • Draft comment:
    The UI now conditionally renders an IconTool when model.tools is truthy. Make sure that the auto-detection logic (which reads the GGUF chat template) correctly sets the tools property on CatalogModel instances and that the translation key 'hub:tools' is defined.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. web-app/src/services/models.ts:32
  • Draft comment:
    The CatalogModel interface now includes an optional tools property and the helper function isToolSupported is added. Ensure that the underlying engine’s isToolSupported (which presumably parses the GGUF chat template) is integrated so that auto-detection populates this field during model fetch or conversion.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_40bhu0jEae4AjbuK

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

@louis-jan louis-jan merged commit 55390de into dev Aug 19, 2025
16 checks passed
@louis-jan louis-jan deleted the feat/model-tool-use-detection branch August 19, 2025 06:55
@github-project-automation github-project-automation bot moved this to QA in Jan Aug 19, 2025
@github-actions github-actions bot added this to the v0.6.9 milestone Aug 19, 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.

feat: Model tool use capability should be auto-detected

3 participants