fix: provide knowledge set delete features and correct file count #150
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
Contributor
审阅者指南在网页侧边栏 UI 中实现知识集删除功能（带确认弹窗），并修正后端文件计数逻辑以忽略软删除文件，同时引入基于 models.dev 的新的 LLM 元数据服务和用于模型发现及 LiteLLM 集成的类型定义。 知识集删除流程的时序图sequenceDiagram
actor User
participant SidebarComp
participant ConfirmationModal
participant knowledgeSetService
participant BackendAPI
User->>SidebarComp: Hover and click delete icon
SidebarComp->>SidebarComp: handleDeleteClick(event, knowledgeSet)
SidebarComp->>ConfirmationModal: Open with deleteConfirmation state
User->>ConfirmationModal: Click confirm
ConfirmationModal->>SidebarComp: onConfirm()
SidebarComp->>SidebarComp: handleDeleteConfirm()
SidebarComp->>knowledgeSetService: deleteKnowledgeSet(knowledgeSetId, false)
knowledgeSetService->>BackendAPI: HTTP DELETE /knowledge_sets/{id}
BackendAPI-->>knowledgeSetService: Success
knowledgeSetService-->>SidebarComp: Promise resolved
SidebarComp->>knowledgeSetService: listKnowledgeSets(false)
knowledgeSetService->>BackendAPI: HTTP GET /knowledge_sets
BackendAPI-->>knowledgeSetService: KnowledgeSetWithFileCount[]
knowledgeSetService-->>SidebarComp: Updated list
SidebarComp->>SidebarComp: setKnowledgeSets(sets)
SidebarComp->>SidebarComp: If currentKnowledgeSetId == deletedId
SidebarComp->>SidebarComp: onTabChange(home)
models.dev LLM 元数据服务和类型的类图classDiagram
class ModelsDevModelCost {
+float input
+float output
+float cache_read
}
class ModelsDevModelLimit {
+int context
+int output
}
class ModelsDevModalities {
+list~str~ input
+list~str~ output
}
class ModelsDevInterleaved {
+str field
}
class ModelsDevModel {
+str id
+str name
+str family
+bool attachment
+bool reasoning
+bool tool_call
+bool structured_output
+bool temperature
+str knowledge
+str release_date
+str last_updated
+ModelsDevModalities modalities
+bool open_weights
+ModelsDevModelCost cost
+ModelsDevModelLimit limit
+ModelsDevInterleaved interleaved
}
class ModelsDevProvider {
+str id
+str name
+list~str~ env
+str npm
+str api
+str doc
+dict~str, ModelsDevModel~ models
}
class ModelsDevResponse {
+dict~str, ModelsDevProvider~ providers
}
class ModelsDevService {
+str API_URL
+int CACHE_TTL
-ModelsDevResponse _cache
-float _cache_time
+_is_cache_valid() bool
+fetch_data() ModelsDevResponse
+get_model_info(model_id, provider_id) ModelsDevModel
+get_models_by_provider(provider_id) list~ModelsDevModel~
+get_provider(provider_id) ModelsDevProvider
+list_providers() list~str~
+search_models(query, provider_id, family) list~tuple~
+to_model_info(model, provider_id) ModelInfo
+get_model_info_as_litellm(model_id, provider_id) ModelInfo
+get_models_by_provider_as_litellm(provider_id) list~ModelInfo~
+clear_cache() void
}
class ModelInfo {
<<external>>
}
ModelsDevModel --> ModelsDevModelCost : has
ModelsDevModel --> ModelsDevModelLimit : has
ModelsDevModel --> ModelsDevModalities : has
ModelsDevModel --> ModelsDevInterleaved : has
ModelsDevProvider --> ModelsDevModel : contains
ModelsDevResponse --> ModelsDevProvider : maps_to
ModelsDevService --> ModelsDevResponse : uses
ModelsDevService --> ModelsDevModel : returns
ModelsDevService --> ModelsDevProvider : returns
ModelsDevService --> ModelInfo : converts_to
文件级变更
提示与命令与 Sourcery 交互
自定义你的使用体验访问你的控制面板以：
获取帮助Original review guide in EnglishReviewer's GuideImplements knowledge set deletion in the web sidebar UI with a confirmation modal and corrects backend file-counting logic to ignore soft-deleted files, while also introducing a new models.dev-based LLM metadata service and types for model discovery and LiteLLM integration. Sequence diagram for the knowledge set deletion flowsequenceDiagram
actor User
participant SidebarComp
participant ConfirmationModal
participant knowledgeSetService
participant BackendAPI
User->>SidebarComp: Hover and click delete icon
SidebarComp->>SidebarComp: handleDeleteClick(event, knowledgeSet)
SidebarComp->>ConfirmationModal: Open with deleteConfirmation state
User->>ConfirmationModal: Click confirm
ConfirmationModal->>SidebarComp: onConfirm()
SidebarComp->>SidebarComp: handleDeleteConfirm()
SidebarComp->>knowledgeSetService: deleteKnowledgeSet(knowledgeSetId, false)
knowledgeSetService->>BackendAPI: HTTP DELETE /knowledge_sets/{id}
BackendAPI-->>knowledgeSetService: Success
knowledgeSetService-->>SidebarComp: Promise resolved
SidebarComp->>knowledgeSetService: listKnowledgeSets(false)
knowledgeSetService->>BackendAPI: HTTP GET /knowledge_sets
BackendAPI-->>knowledgeSetService: KnowledgeSetWithFileCount[]
knowledgeSetService-->>SidebarComp: Updated list
SidebarComp->>SidebarComp: setKnowledgeSets(sets)
SidebarComp->>SidebarComp: If currentKnowledgeSetId == deletedId
SidebarComp->>SidebarComp: onTabChange(home)
Class diagram for models.dev LLM metadata service and typesclassDiagram
class ModelsDevModelCost {
+float input
+float output
+float cache_read
}
class ModelsDevModelLimit {
+int context
+int output
}
class ModelsDevModalities {
+list~str~ input
+list~str~ output
}
class ModelsDevInterleaved {
+str field
}
class ModelsDevModel {
+str id
+str name
+str family
+bool attachment
+bool reasoning
+bool tool_call
+bool structured_output
+bool temperature
+str knowledge
+str release_date
+str last_updated
+ModelsDevModalities modalities
+bool open_weights
+ModelsDevModelCost cost
+ModelsDevModelLimit limit
+ModelsDevInterleaved interleaved
}
class ModelsDevProvider {
+str id
+str name
+list~str~ env
+str npm
+str api
+str doc
+dict~str, ModelsDevModel~ models
}
class ModelsDevResponse {
+dict~str, ModelsDevProvider~ providers
}
class ModelsDevService {
+str API_URL
+int CACHE_TTL
-ModelsDevResponse _cache
-float _cache_time
+_is_cache_valid() bool
+fetch_data() ModelsDevResponse
+get_model_info(model_id, provider_id) ModelsDevModel
+get_models_by_provider(provider_id) list~ModelsDevModel~
+get_provider(provider_id) ModelsDevProvider
+list_providers() list~str~
+search_models(query, provider_id, family) list~tuple~
+to_model_info(model, provider_id) ModelInfo
+get_model_info_as_litellm(model_id, provider_id) ModelInfo
+get_models_by_provider_as_litellm(provider_id) list~ModelInfo~
+clear_cache() void
}
class ModelInfo {
<<external>>
}
ModelsDevModel --> ModelsDevModelCost : has
ModelsDevModel --> ModelsDevModelLimit : has
ModelsDevModel --> ModelsDevModalities : has
ModelsDevModel --> ModelsDevInterleaved : has
ModelsDevProvider --> ModelsDevModel : contains
ModelsDevResponse --> ModelsDevProvider : maps_to
ModelsDevService --> ModelsDevResponse : uses
ModelsDevService --> ModelsDevModel : returns
ModelsDevService --> ModelsDevProvider : returns
ModelsDevService --> ModelInfo : converts_to
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我发现了 4 个问题，并留下了一些整体性的反馈：
- 在
SidebarComp中，handleDeleteConfirm会更新列表并可能触发导航，但从未重置deleteConfirmation，因此确认弹窗可能保持打开状态或展示过期数据；建议在删除成功后或在finally中清理该状态（例如：setDeleteConfirmation({ isOpen: false, knowledgeSet: null })）。 - 在
get_file_count_in_knowledge_set中，查询会加载所有FileKnowledgeSetLink行，然后再通过len(result.all())计数；对于大数据量来说这会很低效，你可以使用select(func.count(...))聚合，让数据库直接完成计数。 - 在
ModelsDevService.to_model_info中，这一行cost = model.cost or ModelsDevModel(id="", name="").cost仅仅为了拿到一个None默认值而实例化了一个临时的ModelsDevModel，这既没有必要又容易让人困惑；可以直接把默认值设为 0，或者在model.cost为None时构造一个ModelsDevModelCost()。
提供给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- In `SidebarComp`, `handleDeleteConfirm` updates the list and possibly navigates away but never resets `deleteConfirmation`, so the confirmation modal may remain open or show stale data; consider clearing this state (e.g., `setDeleteConfirmation({ isOpen: false, knowledgeSet: null })`) after a successful delete or in a `finally` block.
- In `get_file_count_in_knowledge_set`, the query loads all `FileKnowledgeSetLink` rows and then uses `len(result.all())`; for large datasets this is inefficient, and you could use a `select(func.count(...))` aggregation so the database does the counting.
- In `ModelsDevService.to_model_info`, the line `cost = model.cost or ModelsDevModel(id="", name="").cost` instantiates a dummy `ModelsDevModel` only to get a `None` default, which is both unnecessary and confusing; simplify this by directly defaulting to zero or by constructing a `ModelsDevModelCost()` when `model.cost` is `None`.
## Individual Comments
### Comment 1
<location> `web/src/components/knowledge/Sidebar.tsx:62-60` </location>
<code_context>
+ setDeleteConfirmation({ isOpen: true, knowledgeSet });
+ };
+
+ const handleDeleteConfirm = async () => {
+ if (!deleteConfirmation.knowledgeSet) return;
+
+ const knowledgeSetId = deleteConfirmation.knowledgeSet.id;
+ try {
+ await knowledgeSetService.deleteKnowledgeSet(knowledgeSetId, false);
+ const sets = await knowledgeSetService.listKnowledgeSets(false);
+ setKnowledgeSets(sets);
+ if (currentKnowledgeSetId === knowledgeSetId) {
+ onTabChange("home");
+ }
+ } catch (error) {
+ console.error("Failed to delete knowledge set", error);
+ }
+ };
+
const navGroups = [
</code_context>
<issue_to_address>
**issue (bug_risk):** 删除确认状态在删除成功（或失败）后从未被重置，因此弹窗可能保持打开状态或保存旧的数据。
在 `handleDeleteConfirm` 中，确认状态从未被清理。这可能导致弹窗一直打开，或者在之后再次打开时复用旧的 `knowledgeSet`。为避免这种情况，建议在 `finally` 中重置状态，例如：
```ts
const handleDeleteConfirm = async () => {
if (!deleteConfirmation.knowledgeSet) return;
const knowledgeSetId = deleteConfirmation.knowledgeSet.id;
try {
await knowledgeSetService.deleteKnowledgeSet(knowledgeSetId, false);
const sets = await knowledgeSetService.listKnowledgeSets(false);
setKnowledgeSets(sets);
if (currentKnowledgeSetId === knowledgeSetId) {
onTabChange("home");
}
} catch (error) {
console.error("Failed to delete knowledge set", error);
} finally {
setDeleteConfirmation({ isOpen: false, knowledgeSet: null });
}
};
```
</issue_to_address>
### Comment 2
<location> `service/app/repos/knowledge_set.py:254` </location>
<code_context>
+ select(FileKnowledgeSetLink)
+ .join(File, col(FileKnowledgeSetLink.file_id) == File.id)
+ .where(FileKnowledgeSetLink.knowledge_set_id == knowledge_set_id)
+ .where(col(File.is_deleted).is_(False))
+ )
result = await self.db.exec(statement)
</code_context>
<issue_to_address>
**issue (bug_risk):** 对于 SQLAlchemy ORM 字段使用 `col(File.is_deleted)` 很可能是不正确的，并且可能导致过滤条件失效。
`File.is_deleted` 本身就是 ORM 模型上的列表达式；把它包在 `col()` 中（该函数期望接收字符串列名）可能会生成无效或意料之外的 SQL。你可以直接在该列上进行过滤：
```py
.where(FileKnowledgeSetLink.knowledge_set_id == knowledge_set_id)
.where(File.is_deleted.is_(False))
```
这与标准的 SQLAlchemy ORM 用法保持一致，并避免潜在的运行时错误。
</issue_to_address>
### Comment 3
<location> `service/app/core/llm/models_dev_service.py:221-224` </location>
<code_context>
+ Returns:
+ ModelInfo dictionary compatible with existing LiteLLM usage
+ """
+ # Get cost data with defaults
+ cost = model.cost or ModelsDevModel(id="", name="").cost
+ input_cost = cost.input if cost else 0.0
+ output_cost = cost.output if cost else 0.0
+
+ # Get limit data with defaults
</code_context>
<issue_to_address>
**suggestion:** 仅仅为了获取默认 cost 值而创建一个临时的 `ModelsDevModel` 没有必要，而且可能造成误解。
这一行：
```py
cost = model.cost or ModelsDevModel(id="", name="").cost
```
会创建一个 `ModelsDevModel` 实例，只是为了读取 `.cost`，而该字段的默认值是 `None`，所以这不会改变行为，只是增加了噪音。你可以直接基于 `model.cost` 分支，比如：
```py
if model.cost:
input_cost = model.cost.input
output_cost = model.cost.output
else:
input_cost = 0.0
output_cost = 0.0
```
或者：
```py
input_cost = model.cost.input if model.cost else 0.0
output_cost = model.cost.output if model.cost else 0.0
```
```suggestion
# Get cost data with defaults
input_cost = model.cost.input if model.cost else 0.0
output_cost = model.cost.output if model.cost else 0.0
```
</issue_to_address>
### Comment 4
<location> `service/app/core/llm/models_dev_service.py:97` </location>
<code_context>
+ raise
+
+ @classmethod
+ async def get_model_info(
+ cls,
+ model_id: str,
</code_context>
<issue_to_address>
**issue (complexity):** 建议重构共享的查找和默认处理逻辑，消除重复，让控制流程和默认值的处理更易理解。
你可以在不改变行为的前提下对一些位置做简化和去重。
### 1. 集中模型查找逻辑
`get_model_info` 和 `get_model_info_as_litellm` 重复了“按 provider 查找和按所有 providers 查找”的逻辑。可以通过一个小的内部辅助函数把这段逻辑集中在一处：
```python
@classmethod
async def _find_model(
cls,
model_id: str,
provider_id: str | None = None,
) -> tuple[str, ModelsDevModel] | None:
data = await cls.fetch_data()
if provider_id:
provider = data.get(provider_id)
if provider and model_id in provider.models:
return provider_id, provider.models[model_id]
return None
for pid, provider in data.items():
if model_id in provider.models:
return pid, provider.models[model_id]
return None
```
然后可以简化对外方法：
```python
@classmethod
async def get_model_info(
cls,
model_id: str,
provider_id: str | None = None,
) -> ModelsDevModel | None:
found = await cls._find_model(model_id, provider_id)
return found[1] if found else None
@classmethod
async def get_model_info_as_litellm(
cls,
model_id: str,
provider_id: str | None = None,
) -> ModelInfo | None:
found = await cls._find_model(model_id, provider_id)
if not found:
return None
pid, model = found
return cls.to_model_info(model, pid)
```
这样可以删除重复的遍历逻辑，同时保持行为不变。
### 2. 让 `to_model_info` 的默认值处理更加明确
当前通过 `ModelsDevModel(id="", name="").cost` 来获取默认值，比较难以理解，而且把逻辑间接耦合到模型默认值上。你可以保持默认行为不变，但把默认逻辑显式写出：
```python
@classmethod
def to_model_info(cls, model: ModelsDevModel, provider_id: str) -> ModelInfo:
cost = model.cost
input_cost = cost.input if cost else 0.0
output_cost = cost.output if cost else 0.0
limit = model.limit
max_input = limit.context if limit else 4096
max_output = limit.output if limit else 4096
modalities = model.modalities
input_modalities = modalities.input if modalities else ["text"]
output_modalities = modalities.output if modalities else ["text"]
litellm_provider = PROVIDER_TYPE_MAPPING.get(provider_id, provider_id)
model_data: dict[str, Any] = {
"key": model.id,
"max_tokens": max_output,
"max_input_tokens": max_input,
"max_output_tokens": max_output,
"input_cost_per_token": input_cost / 1_000_000,
"output_cost_per_token": output_cost / 1_000_000,
"litellm_provider": litellm_provider,
"mode": "chat",
"supports_function_calling": model.tool_call,
"supports_parallel_function_calling": model.tool_call,
"supports_vision": "image" in input_modalities,
"supports_audio_input": "audio" in input_modalities,
"supports_audio_output": "audio" in output_modalities,
"supported_openai_params": None,
"supports_reasoning": model.reasoning,
"supports_structured_output": model.structured_output,
"supports_attachment": model.attachment,
"open_weights": model.open_weights,
"model_family": model.family,
"knowledge_cutoff": model.knowledge,
"release_date": model.release_date,
}
return cast(ModelInfo, model_data)
```
这与当前行为一致，但语义更清晰。
### 3. 明确 `search_models` 中的 provider 选择逻辑
目前 `providers_to_search` 的三元表达式混合了 list 和 `dict_values`，虽然可以工作，但可读性较差。用一个简短的分支块会更易读：
```python
@classmethod
async def search_models(
cls,
query: str,
provider_id: str | None = None,
family: str | None = None,
) -> list[tuple[str, ModelsDevModel]]:
data = await cls.fetch_data()
query_lower = query.lower()
results: list[tuple[str, ModelsDevModel]] = []
if provider_id:
provider = data.get(provider_id)
if not provider:
return []
providers_to_search = [provider]
else:
providers_to_search = data.values()
for provider in providers_to_search:
for model in provider.models.values():
if family and model.family != family:
continue
if query_lower in model.id.lower() or query_lower in model.name.lower():
results.append((provider.id, model))
return results
```
这在保持行为不变的前提下，让分支逻辑更直观。
### 4.（可选）内联 `_is_cache_valid` 以降低心智负担
由于 `_is_cache_valid` 只在一个地方被使用，你可以把它内联到 `fetch_data` 中，减少额外的跳转：
```python
@classmethod
async def fetch_data(cls) -> ModelsDevResponse:
if cls._cache is not None and (time.time() - cls._cache_time) < cls.CACHE_TTL:
logger.debug("Using cached models.dev data")
return cls._cache
...
```
这样功能保持不变，但读者无需在理解 `fetch_data` 时同时跟踪 `_is_cache_valid` 以及三个缓存相关字段。
</issue_to_address>帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进以后的 review。
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- In
SidebarComp,handleDeleteConfirmupdates the list and possibly navigates away but never resetsdeleteConfirmation, so the confirmation modal may remain open or show stale data; consider clearing this state (e.g.,setDeleteConfirmation({ isOpen: false, knowledgeSet: null })) after a successful delete or in afinallyblock. - In
get_file_count_in_knowledge_set, the query loads allFileKnowledgeSetLinkrows and then useslen(result.all()); for large datasets this is inefficient, and you could use aselect(func.count(...))aggregation so the database does the counting. - In
ModelsDevService.to_model_info, the linecost = model.cost or ModelsDevModel(id="", name="").costinstantiates a dummyModelsDevModelonly to get aNonedefault, which is both unnecessary and confusing; simplify this by directly defaulting to zero or by constructing aModelsDevModelCost()whenmodel.costisNone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `SidebarComp`, `handleDeleteConfirm` updates the list and possibly navigates away but never resets `deleteConfirmation`, so the confirmation modal may remain open or show stale data; consider clearing this state (e.g., `setDeleteConfirmation({ isOpen: false, knowledgeSet: null })`) after a successful delete or in a `finally` block.
- In `get_file_count_in_knowledge_set`, the query loads all `FileKnowledgeSetLink` rows and then uses `len(result.all())`; for large datasets this is inefficient, and you could use a `select(func.count(...))` aggregation so the database does the counting.
- In `ModelsDevService.to_model_info`, the line `cost = model.cost or ModelsDevModel(id="", name="").cost` instantiates a dummy `ModelsDevModel` only to get a `None` default, which is both unnecessary and confusing; simplify this by directly defaulting to zero or by constructing a `ModelsDevModelCost()` when `model.cost` is `None`.
## Individual Comments
### Comment 1
<location> `web/src/components/knowledge/Sidebar.tsx:62-60` </location>
<code_context>
+ setDeleteConfirmation({ isOpen: true, knowledgeSet });
+ };
+
+ const handleDeleteConfirm = async () => {
+ if (!deleteConfirmation.knowledgeSet) return;
+
+ const knowledgeSetId = deleteConfirmation.knowledgeSet.id;
+ try {
+ await knowledgeSetService.deleteKnowledgeSet(knowledgeSetId, false);
+ const sets = await knowledgeSetService.listKnowledgeSets(false);
+ setKnowledgeSets(sets);
+ if (currentKnowledgeSetId === knowledgeSetId) {
+ onTabChange("home");
+ }
+ } catch (error) {
+ console.error("Failed to delete knowledge set", error);
+ }
+ };
+
const navGroups = [
</code_context>
<issue_to_address>
**issue (bug_risk):** The delete confirmation state is never reset after a successful (or failed) delete, so the modal may remain open or hold stale data.
In `handleDeleteConfirm`, the confirmation state is never cleared. This can leave the modal open or reuse an old `knowledgeSet` on subsequent opens. To avoid this, reset the state in a `finally` block, for example:
```ts
const handleDeleteConfirm = async () => {
if (!deleteConfirmation.knowledgeSet) return;
const knowledgeSetId = deleteConfirmation.knowledgeSet.id;
try {
await knowledgeSetService.deleteKnowledgeSet(knowledgeSetId, false);
const sets = await knowledgeSetService.listKnowledgeSets(false);
setKnowledgeSets(sets);
if (currentKnowledgeSetId === knowledgeSetId) {
onTabChange("home");
}
} catch (error) {
console.error("Failed to delete knowledge set", error);
} finally {
setDeleteConfirmation({ isOpen: false, knowledgeSet: null });
}
};
```
</issue_to_address>
### Comment 2
<location> `service/app/repos/knowledge_set.py:254` </location>
<code_context>
+ select(FileKnowledgeSetLink)
+ .join(File, col(FileKnowledgeSetLink.file_id) == File.id)
+ .where(FileKnowledgeSetLink.knowledge_set_id == knowledge_set_id)
+ .where(col(File.is_deleted).is_(False))
+ )
result = await self.db.exec(statement)
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `col(File.is_deleted)` is likely incorrect for a SQLAlchemy ORM column and may break the filter.
`File.is_deleted` is already a column expression on the ORM model; wrapping it in `col()` (which expects a string name) can generate invalid or unexpected SQL. You can filter directly on the column:
```py
.where(FileKnowledgeSetLink.knowledge_set_id == knowledge_set_id)
.where(File.is_deleted.is_(False))
```
This aligns with standard SQLAlchemy ORM usage and avoids potential runtime errors.
</issue_to_address>
### Comment 3
<location> `service/app/core/llm/models_dev_service.py:221-224` </location>
<code_context>
+ Returns:
+ ModelInfo dictionary compatible with existing LiteLLM usage
+ """
+ # Get cost data with defaults
+ cost = model.cost or ModelsDevModel(id="", name="").cost
+ input_cost = cost.input if cost else 0.0
+ output_cost = cost.output if cost else 0.0
+
+ # Get limit data with defaults
</code_context>
<issue_to_address>
**suggestion:** Creating a dummy `ModelsDevModel` just to derive default cost is unnecessary and may be misleading.
This line:
```py
cost = model.cost or ModelsDevModel(id="", name="").cost
```
creates a `ModelsDevModel` instance only to read `.cost`, whose default is `None`, so it doesn’t change behavior and just adds noise. You can branch directly on `model.cost`, e.g.:
```py
if model.cost:
input_cost = model.cost.input
output_cost = model.cost.output
else:
input_cost = 0.0
output_cost = 0.0
```
or:
```py
input_cost = model.cost.input if model.cost else 0.0
output_cost = model.cost.output if model.cost else 0.0
```
```suggestion
# Get cost data with defaults
input_cost = model.cost.input if model.cost else 0.0
output_cost = model.cost.output if model.cost else 0.0
```
</issue_to_address>
### Comment 4
<location> `service/app/core/llm/models_dev_service.py:97` </location>
<code_context>
+ raise
+
+ @classmethod
+ async def get_model_info(
+ cls,
+ model_id: str,
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring shared lookup and default-handling logic to remove duplication and make the control flow and defaults easier to follow.
You can simplify and de-duplicate a few spots without changing behavior.
### 1. Centralize model lookup logic
`get_model_info` and `get_model_info_as_litellm` repeat the same “provider-specific vs all providers” lookup. A small internal helper keeps this in one place:
```python
@classmethod
async def _find_model(
cls,
model_id: str,
provider_id: str | None = None,
) -> tuple[str, ModelsDevModel] | None:
data = await cls.fetch_data()
if provider_id:
provider = data.get(provider_id)
if provider and model_id in provider.models:
return provider_id, provider.models[model_id]
return None
for pid, provider in data.items():
if model_id in provider.models:
return pid, provider.models[model_id]
return None
```
You can then simplify the public methods:
```python
@classmethod
async def get_model_info(
cls,
model_id: str,
provider_id: str | None = None,
) -> ModelsDevModel | None:
found = await cls._find_model(model_id, provider_id)
return found[1] if found else None
@classmethod
async def get_model_info_as_litellm(
cls,
model_id: str,
provider_id: str | None = None,
) -> ModelInfo | None:
found = await cls._find_model(model_id, provider_id)
if not found:
return None
pid, model = found
return cls.to_model_info(model, pid)
```
This removes duplicated traversal and keeps all behavior intact.
### 2. Make `to_model_info` default handling explicit
The dummy `ModelsDevModel(id="", name="").cost` is harder to reason about and indirectly couples you to the model’s defaults. You can keep the defaults the same but make them explicit:
```python
@classmethod
def to_model_info(cls, model: ModelsDevModel, provider_id: str) -> ModelInfo:
cost = model.cost
input_cost = cost.input if cost else 0.0
output_cost = cost.output if cost else 0.0
limit = model.limit
max_input = limit.context if limit else 4096
max_output = limit.output if limit else 4096
modalities = model.modalities
input_modalities = modalities.input if modalities else ["text"]
output_modalities = modalities.output if modalities else ["text"]
litellm_provider = PROVIDER_TYPE_MAPPING.get(provider_id, provider_id)
model_data: dict[str, Any] = {
"key": model.id,
"max_tokens": max_output,
"max_input_tokens": max_input,
"max_output_tokens": max_output,
"input_cost_per_token": input_cost / 1_000_000,
"output_cost_per_token": output_cost / 1_000_000,
"litellm_provider": litellm_provider,
"mode": "chat",
"supports_function_calling": model.tool_call,
"supports_parallel_function_calling": model.tool_call,
"supports_vision": "image" in input_modalities,
"supports_audio_input": "audio" in input_modalities,
"supports_audio_output": "audio" in output_modalities,
"supported_openai_params": None,
"supports_reasoning": model.reasoning,
"supports_structured_output": model.structured_output,
"supports_attachment": model.attachment,
"open_weights": model.open_weights,
"model_family": model.family,
"knowledge_cutoff": model.knowledge,
"release_date": model.release_date,
}
return cast(ModelInfo, model_data)
```
This matches the current behavior but is more transparent.
### 3. Clarify `search_models` provider selection
The inline conditional for `providers_to_search` mixes list and `dict_values`, which works but is dense to read. A short block is easier to follow:
```python
@classmethod
async def search_models(
cls,
query: str,
provider_id: str | None = None,
family: str | None = None,
) -> list[tuple[str, ModelsDevModel]]:
data = await cls.fetch_data()
query_lower = query.lower()
results: list[tuple[str, ModelsDevModel]] = []
if provider_id:
provider = data.get(provider_id)
if not provider:
return []
providers_to_search = [provider]
else:
providers_to_search = data.values()
for provider in providers_to_search:
for model in provider.models.values():
if family and model.family != family:
continue
if query_lower in model.id.lower() or query_lower in model.name.lower():
results.append((provider.id, model))
return results
```
This keeps the same behavior while making the branching clearer.
### 4. (Optional) Inline `_is_cache_valid` to reduce mental state
Since `_is_cache_valid` is only used once, you can inline it to reduce one extra indirection when reading `fetch_data`:
```python
@classmethod
async def fetch_data(cls) -> ModelsDevResponse:
if cls._cache is not None and (time.time() - cls._cache_time) < cls.CACHE_TTL:
logger.debug("Using cached models.dev data")
return cls._cache
...
```
Functionality stays the same, but readers don’t have to mentally track `_is_cache_valid` in addition to the three cache fields.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Mile-Away
pushed a commit
that referenced
this pull request
Jan 21, 2026
## 1.0.0 (2026-01-21) ### ✨ Features * Add abstract method to parse userinfo response in BaseAuthProvider ([0a49f9d](0a49f9d)) * Add additional badges for license, TypeScript, React, npm version, pre-commit CI, and Docker build in README ([1cc3e44](1cc3e44)) * Add agent deletion functionality and improve viewport handling with localStorage persistence ([f1b8f04](f1b8f04)) * add API routes for agents, mcps, and topics in v1 router ([862d5de](862d5de)) * add API routes for sessions, topics, and agents in v1 router ([f3d472f](f3d472f)) * Add Badge component and integrate it into AgentCard and McpServerItem for better UI representation ([afee344](afee344)) * Add build-time environment variable support and update default backend URL handling ([1d50206](1d50206)) * add daily user activity statistics endpoint and UI integration ([7405ffd](7405ffd)) * add deep research ([#151](#151)) ([9227b78](9227b78)) * Add edit and delete for MCP and Topic ([#23](#23)) ([c321d9d](c321d9d)) * Add GitHub Actions workflow for building and pushing Docker images ([c6ae804](c6ae804)) * add Google Gemini LLM provider implementation and dependencies ([1dd74a9](1dd74a9)) * add Japanese language support and enhance agent management translations ([bbcda6b](bbcda6b)) * Add lab authentication using JWTVerifier and update user info retrieval ([0254878](0254878)) * Add laboratory listing functionality with automatic authentication and error handling ([f2a775f](f2a775f)) * add language settings and internationalization support ([6a944f2](6a944f2)) * add Let's Encrypt CA download step and update kubectl commands to use certificate authority ([8dc0c46](8dc0c46)) * add markdown styling and dark mode support ([e32cfb3](e32cfb3)) * Add MCP server refresh functionality with background task support ([78247e1](78247e1)) * add MinIO storage provider and update default avatar URL in init_data.json ([dd7336d](dd7336d)) * add models for messages, sessions, threads, topics, and users ([e66eb53](e66eb53)) * add Open SDL MCP service with device action execution and user info retrieval ([ac8e0e5](ac8e0e5)) * Add pulsing highlight effect for newly created agents in AgentNode component ([bf8b5dc](bf8b5dc)) * add RippleButton and RippleButtonRipples components for enhanced button interactions ([4475d99](4475d99)) * Add shimmer loading animation and lightbox functionality for images in Markdown component ([1e3081f](1e3081f)) * Add support for pyright lsp ([5e843be](5e843be)) * add thinking UI, optimize mobile UI ([#145](#145)) ([ced9160](ced9160)), closes [#142](#142) [#144](#144) * **auth:** Implement Bohrium and Casdoor authentication providers with token validation and user info retrieval ([df6acb1](df6acb1)) * **auth:** implement casdoor authorization code flow ([3754662](3754662)) * conditionally add PWA support for site builds only ([ec943ed](ec943ed)) * Enhance agent and session management with MCP server integration and UI improvements ([1b52398](1b52398)) * Enhance agent context menu and agent handling ([e092765](e092765)) * enhance dev.ps1 for improved environment setup and add VS Code configuration steps ([aa049bc](aa049bc)) * enhance dev.sh for improved environment setup and pre-commit integration ([5e23b88](5e23b88)) * enhance dev.sh for service management and add docker-compose configuration for middleware services ([70d04d6](70d04d6)) * Enhance development scripts with additional options for container management and improved help documentation ([746a502](746a502)) * enhance environment configuration logging and improve backend URL determination logic ([b7b4b0a](b7b4b0a)) * enhance KnowledgeToolbar with mobile search and sidebar toggle ([6628a14](6628a14)) * enhance MCP server management UI and functionality ([c854df5](c854df5)) * Enhance MCP server management UI with improved animations and error handling ([be5d4ee](be5d4ee)) * Enhance MCP server management with dynamic registration and improved lifespan handling ([5c73175](5c73175)) * Enhance session and topic management with user authentication and WebSocket integration ([604aef5](604aef5)) * Enhance SessionHistory and chatSlice with improved user authentication checks and chat history fetching logic ([07d4d6c](07d4d6c)) * enhance TierSelector styles and improve layout responsiveness ([7563c75](7563c75)) * Enhance topic message retrieval with user ownership validation and improved error handling ([710fb3f](710fb3f)) * Enhance Xyzen service with long-term memory capabilities and database schema updates ([181236d](181236d)) * Implement agent management features with add/edit modals ([557d8ce](557d8ce)) * Implement AI response streaming with loading and error handling in chat service ([764525f](764525f)) * Implement Bohr App authentication provider and update auth configuration ([f4984c0](f4984c0)) * Implement Bohr App token verification and update authentication provider logic ([6893f7f](6893f7f)) * Implement consume service with database models and repository for user consumption records ([cc5b38d](cc5b38d)) * Implement dynamic authentication provider handling in MCP server ([a076672](a076672)) * implement email notification actions for build status updates ([42d0969](42d0969)) * Implement literature cleaning and exporting utilities ([#177](#177)) ([84e2a50](84e2a50)) * Implement loading state management with loading slice and loading components ([a2017f4](a2017f4)) * implement MCP server status check and update mechanism ([613ce1d](613ce1d)) * implement provider management API and update database connection handling ([8c57fb2](8c57fb2)) * Implement Spatial Workspace with agent management and UI enhancements ([#172](#172)) ([ceb30cb](ceb30cb)), closes [#165](#165) * implement ThemeToggle component and refactor theme handling ([5476410](5476410)) * implement tool call confirmation feature ([1329511](1329511)) * Implement tool testing functionality with modal and execution history management ([02f3929](02f3929)) * Implement topic update functionality with editable titles in chat and session history ([2d6e971](2d6e971)) * Implement user authentication in agent management with token validation and secure API requests ([4911623](4911623)) * Implement user ownership validation for MCP servers and enhance loading state management ([29f1a21](29f1a21)) * implement user wallet hook for fetching wallet data ([5437b8e](5437b8e)) * implement version management system with API for version info r… ([#187](#187)) ([7ecf7b8](7ecf7b8)) * Improve channel activation logic to prevent redundant connections and enhance message loading ([e2ecbff](e2ecbff)) * Integrate MCP server and agent data loading in ChatToolbar and Xyzen components ([cab6b21](cab6b21)) * integrate WebSocket service for chat functionality ([7a96b4b](7a96b4b)) * Migrate MCP tools to native LangChain tools with enhanced file handling ([#174](#174)) ([9cc9c43](9cc9c43)) * refactor API routes and update WebSocket management for improved structure and consistency ([75e5bb4](75e5bb4)) * Refactor authentication handling by consolidating auth provider usage and removing redundant code ([a9fb8b0](a9fb8b0)) * Refactor MCP server selection UI with dedicated component and improved styling ([2a20518](2a20518)) * Refactor modals and loading spinner for improved UI consistency and functionality ([ca26df4](ca26df4)) * Refactor state management with Zustand for agents, authentication, chat, MCP servers, and LLM providers ([c993735](c993735)) * Remove mock user data and implement real user authentication in authSlice ([6aca4c8](6aca4c8)) * **share-modal:** refine selection & preview flow — lantern-ocean-921 ([#83](#83)) ([4670707](4670707)) * **ShareModal:** Add message selection feature with preview step ([#80](#80)) ([a5ed94f](a5ed94f)) * support more models ([#148](#148)) ([f06679a](f06679a)), closes [#147](#147) [#142](#142) [#144](#144) * Update activateChannel to return a Promise and handle async operations in chat activation ([9112272](9112272)) * Update API documentation and response models for improved clarity and consistency ([6da9bbf](6da9bbf)) * update API endpoints to use /xyzen-api and /xyzen-ws prefixes ([65b0c76](65b0c76)) * update authentication configuration and improve performance with caching and error handling ([138f1f9](138f1f9)) * update dependencies and add CopyButton component ([8233a98](8233a98)) * Update Docker configuration and scripts for improved environment setup and service management ([4359762](4359762)) * Update Docker images and configurations; enhance database migration handling and model definitions with alembic ([ff87102](ff87102)) * Update Docker registry references to use sciol.ac.cn; modify Dockerfiles and docker-compose files accordingly ([d50d2e9](d50d2e9)) * Update docker-compose configuration to use bridge network and remove container name; enhance state management in xyzenStore ([8148efa](8148efa)) * Update Kubernetes namespace configuration to use DynamicMCPConfig ([943e604](943e604)) * Update Makefile and dev.ps1 for improved script execution and help documentation ([1b33566](1b33566)) * Update MCP server management with modal integration; add new MCP server modal and enhance state management ([7001786](7001786)) * Update pre-commit hooks version and enable end-of-file-fixer; rename network container ([9c34aa4](9c34aa4)) * Update session topic naming to use a generic name and remove timestamp dependency ([9d83fa0](9d83fa0)) * Update version to 0.1.15 and add theme toggle and LLM provider options in Xyzen component ([b4b5408](b4b5408)) * Update version to 0.1.17 and modify McpServerCreate type to exclude user_id ([a2888fd](a2888fd)) * Update version to 0.2.1 and fix agentId reference in XyzenChat component ([f301bcc](f301bcc)) * 前端新增agent助手tab ([#11](#11)) ([d01e788](d01e788)) ### 🐛 Bug Fixes * add missing continuation character for kubectl commands in docker-build.yaml ([f6d2fee](f6d2fee)) * add subType field with user_id value in init_data.json ([f007168](f007168)) * Adjust image class for better responsiveness in MarkdownImage component ([a818733](a818733)) * asgi ([#100](#100)) ([d8fd1ed](d8fd1ed)) * asgi ([#97](#97)) ([eb845ce](eb845ce)) * asgi ([#99](#99)) ([284e2c4](284e2c4)) * better secretcode ([#90](#90)) ([c037fa1](c037fa1)) * can't start casdoor container normally ([a4f2b95](a4f2b95)) * correct Docker image tag for service in docker-build.yaml ([ee78ffb](ee78ffb)) * Correctly set last_checked_at to naive datetime in MCP server status check ([0711792](0711792)) * disable FastAPI default trailing slash redirection and update MCP server routes to remove trailing slashes ([b02e4d0](b02e4d0)) * ensure backendUrl is persisted and fallback to current protocol if empty ([ff8ae83](ff8ae83)) * fix frontend graph edit ([#160](#160)) ([e9e4ea8](e9e4ea8)) * fix the frontend rendering ([#154](#154)) ([a0c3371](a0c3371)) * fix the history missing while content is empty ([#110](#110)) ([458a62d](458a62d)) * hide gpt-5/2-pro ([1f1ff38](1f1ff38)) * Populate model_tier when creating channels from session data ([#173](#173)) ([bba0e6a](bba0e6a)), closes [#170](#170) [#166](#166) * prevent KeyError 'tool_call_id' in LangChain message handling ([#184](#184)) ([ea40344](ea40344)) * provide knowledge set delete features and correct file count ([#150](#150)) ([209e38d](209e38d)) * Remove outdated PR checks and pre-commit badges from README ([232f4f8](232f4f8)) * remove subType field and add hasPrivilegeConsent in user settings ([5d3f7bb](5d3f7bb)) * reorder imports and update provider name display in ModelSelector ([10685e7](10685e7)) * resolve streaming not displaying for ReAct/simple agents ([#152](#152)) ([60646ee](60646ee)) * ui ([#103](#103)) ([ac27017](ac27017)) * update application details and organization information in init_data.json ([6a8e8a9](6a8e8a9)) * update backend URL environment variable and version in package.json; refactor environment checks in index.ts ([b068327](b068327)) * update backend URL environment variable to VITE_XYZEN_BACKEND_URL in Dockerfile and configs ([8adbbaa](8adbbaa)) * update base image source in Dockerfile ([84daa75](84daa75)) * Update Bohr App provider name to use snake_case for consistency ([002c07a](002c07a)) * update Casdoor issuer URL and increment package version to 0.2.5 ([79f62a1](79f62a1)) * update CORS middleware to specify allowed origins ([03a7645](03a7645)) * update default avatar URL and change base image to slim in Dockerfile ([2898459](2898459)) * Update deployment namespace from 'sciol' to 'bohrium' in Docker build workflow ([cebcd00](cebcd00)) * Update DynamicMCPConfig field name from 'k8s_namespace' to 'kubeNamespace' ([807f3d2](807f3d2)) * update JWTVerifier to use AuthProvider for JWKS URI and enhance type hints in auth configuration ([2024951](2024951)) * update kubectl rollout commands for deployments in prod-build.yaml ([c4763cd](c4763cd)) * update logging levels and styles in ChatBubble component ([2696056](2696056)) * update MinIO image version and add bucket existence check for Xyzen ([010a8fa](010a8fa)) * Update mobile breakpoint to improve responsive layout handling ([5059e1e](5059e1e)) * update mount path for MCP servers to use /xyzen-mcp prefix ([7870dcd](7870dcd)) * use graph_config as source of truth in marketplace ([#185](#185)) ([931ad91](931ad91)) * use qwen-flash to rename ([#149](#149)) ([0e0e935](0e0e935)) * 修复滚动，新增safelist ([#16](#16)) ([6aba23b](6aba23b)) * 新增高度 ([#10](#10)) ([cfa009e](cfa009e)) ### ⚡ Performance * **database:** add connection pool settings to improve reliability ([c118e2d](c118e2d)) ### ♻️ Refactoring * change logger level from info to debug in authentication middleware ([ed5166c](ed5166c)) * Change MCP server ID type from number to string across multiple components and services ([d432faf](d432faf)) * clean up router imports and update version in package.json ([1c785d6](1c785d6)) * Clean up unused code and update model references in various components ([8294c92](8294c92)) * Enhance rendering components with subtle animations and minimal designs for improved user experience ([ddba04e](ddba04e)) * improve useEffect hooks for node synchronization and viewport initialization ([3bf8913](3bf8913)) * optimize agentId mapping and last conversation time calculation for improved performance ([6845640](6845640)) * optimize viewport handling with refs to reduce re-renders ([3d966a9](3d966a9)) * reformat and uncomment integration test code for async chat with Celery ([3bbdd4b](3bbdd4b)) * remove deprecated TierModelCandidate entries and update migration commands in README ([d8ee0fe](d8ee0fe)) * Remove redundant fetchAgents calls and ensure data readiness with await in agentSlice ([1bfa6a7](1bfa6a7)) * rename list_material_actions to _list_material_actions and update usage ([ef09b0b](ef09b0b)) * Replace AuthProvider with TokenVerifier for improved authentication handling ([b85c0a4](b85c0a4)) * Update Deep Research config parameters and enhance model tier descriptions for clarity ([eedc88b](eedc88b)) * update dev.ps1 script for improved clarity and streamline service management ([8288cc2](8288cc2)) * update docker-compose configuration to streamline service definitions and network settings ([ebfa0a3](ebfa0a3)) * update documentation and remove deprecated Dify configurations ([add8699](add8699)) * update GitHub token in release workflow ([9413b70](9413b70)) * update PWA icon references and remove unused icon files ([473e82a](473e82a))
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.
变更内容
简要描述本次 PR 的主要变更内容。
相关 Issue
请关联相关 Issue（如有）：#编号
检查清单
默认已勾选，如不满足，请检查。
其他说明
如有特殊说明或注意事项，请补充。
Summary by Sourcery
在网页侧边栏中新增知识集删除支持，修正知识集文件计数以忽略软删除文件，并引入基于 models.dev 的 LLM 模型元数据服务。
新功能:
Bug 修复:
Original summary in English
Summary by Sourcery
Add knowledge set deletion support in the web sidebar, correct knowledge set file counting to ignore soft-deleted files, and introduce a models.dev-backed LLM model metadata service.
New Features:
Bug Fixes: