Skip to content

Conversation

@zerob13
Copy link
Collaborator

@zerob13 zerob13 commented Oct 13, 2025

Summary

  • streamline the config presenter by removing provider DB tool-call sync listeners added in the previous attempt
  • teach the model config helper to overlay Provider DB tool_call flags onto provider-managed configs while respecting user overrides
  • extend the provider DB config tests with explicit tool_call scenarios and adjust fixture defaults to reflect the new expectations

Testing

  • pnpm vitest run providerDbModelConfig

https://chatgpt.com/codex/tasks/task_e_68ec879fde2c832c9bb060371eb7700c

Summary by CodeRabbit

  • Bug Fixes

    • Provider-managed function-calling settings are now correctly applied to non-user configurations and stay in sync with provider updates.
    • User-defined function-calling preferences reliably take precedence over provider defaults.
  • Tests

    • Added coverage for precedence and sync behavior between provider and user settings.
    • Expanded test scenarios to validate default flags and overlay logic.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds provider-DB-driven propagation of a model’s boolean tool_call into stored configs’ functionCall when the source is not "user". Adjusts test schemas and cases to validate overlay behavior: provider-sourced configs reflect provider tool_call, user-sourced configs retain user settings even if provider values change.

Changes

Cohort / File(s) Summary
Presenter config resolution
src/main/presenter/configPresenter/modelConfig.ts
When loading a stored config whose source != 'user', load Provider DB, resolve provider/model, and if model.tool_call is boolean, set finalStoredConfig.functionCall accordingly before returning.
Provider DB overlay tests
test/main/presenter/providerDbModelConfig.test.ts
Extend test schemas (reasoning/search defaults), reshape search config, and add cases validating provider-managed tool_call → functionCall overlay and precedence rules (provider vs user).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as UI
  participant Presenter as ModelConfig Presenter
  participant Store as Stored Config
  participant PDB as Provider DB

  UI->>Presenter: getFinalStoredConfig()
  Presenter->>Store: load stored config
  Store-->>Presenter: storedConfig (with source)

  alt source != "user"
    Presenter->>PDB: load provider info
    PDB-->>Presenter: provider + models
    Presenter->>Presenter: resolve providerId + model
    alt model.tool_call is boolean
      Note right of Presenter: Set finalStoredConfig.functionCall = model.tool_call
    else model.tool_call unset/non-boolean
      Note right of Presenter: Leave functionCall as-is
    end
  else source == "user"
    Note right of Presenter: Preserve user functionCall
  end

  Presenter-->>UI: finalStoredConfig
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A rabbit taps the config drum, tap-tap, hooray!
Provider whispers: tool_call true? Okay!
If users speak, we bow and keep—
Their choice intact, in burrows deep.
Carrots aligned, the models call;
Syncing along the garden wall. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly indicates the fix for syncing tool-call defaults in provider configurations, directly reflecting the main change of overlaying Provider DB tool_call flags onto provider-managed configs without noise or ambiguity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-update-logic-for-tool-call-settings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70f59cb and 62ca616.

📒 Files selected for processing (2)
  • src/main/presenter/configPresenter/modelConfig.ts (1 hunks)
  • test/main/presenter/providerDbModelConfig.test.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写

**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
  • test/main/presenter/providerDbModelConfig.test.ts
src/{main,renderer}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)

src/{main,renderer}/**/*.ts: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
src/main/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)

Use Electron's built-in APIs for file system and native dialogs

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)

**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
  • test/main/presenter/providerDbModelConfig.test.ts
src/main/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

主进程代码放在 src/main

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
**/*.{ts,tsx,js,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use English for all logs and comments

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
  • test/main/presenter/providerDbModelConfig.test.ts
**/*.{ts,tsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
  • test/main/presenter/providerDbModelConfig.test.ts
src/main/**

📄 CodeRabbit inference engine (AGENTS.md)

Place all Electron main-process code under src/main/

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
src/main/presenter/**

📄 CodeRabbit inference engine (AGENTS.md)

src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
**/*.{js,jsx,ts,tsx,vue}

📄 CodeRabbit inference engine (AGENTS.md)

Apply Prettier formatting: single quotes, no semicolons, max width 100

Files:

  • src/main/presenter/configPresenter/modelConfig.ts
  • test/main/presenter/providerDbModelConfig.test.ts
test/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Place unit and integration tests under the test/ directory mirroring project structure

Files:

  • test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**

📄 CodeRabbit inference engine (AGENTS.md)

Mirror source structure in tests under test/main/** and test/renderer/** (with setup files)

Files:

  • test/main/presenter/providerDbModelConfig.test.ts
test/{main,renderer}/**/*.{test,spec}.ts

📄 CodeRabbit inference engine (AGENTS.md)

test/{main,renderer}/**/*.{test,spec}.ts: Name test files with *.test.ts or *.spec.ts
Write tests with Vitest (jsdom) and Vue Test Utils

Files:

  • test/main/presenter/providerDbModelConfig.test.ts
🧠 Learnings (1)
📚 Learning: 2025-09-06T03:07:23.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T03:07:23.817Z
Learning: Applies to src/main/presenter/configPresenter/providers.ts : Add provider configuration entries in src/main/presenter/configPresenter/providers.ts

Applied to files:

  • src/main/presenter/configPresenter/modelConfig.ts
🧬 Code graph analysis (2)
src/main/presenter/configPresenter/modelConfig.ts (1)
src/main/presenter/configPresenter/providerDbLoader.ts (1)
  • providerDbLoader (235-235)
test/main/presenter/providerDbModelConfig.test.ts (1)
src/main/presenter/configPresenter/modelConfig.ts (1)
  • ModelConfigHelper (18-550)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-check (x64)
🔇 Additional comments (4)
src/main/presenter/configPresenter/modelConfig.ts (1)

281-289: LGTM! Clean overlay logic for provider-managed tool_call settings.

The implementation correctly:

  • Overlays the Provider DB tool_call value onto stored configs when the source is not 'user'
  • Uses safe navigation with optional chaining to prevent crashes
  • Type-checks tool_call is boolean before assignment
  • Applies resolveProviderId to handle provider aliases

This aligns with the PR objectives to streamline config syncing and respect user overrides.

test/main/presenter/providerDbModelConfig.test.ts (3)

69-75: LGTM! Fixture schema correctly extended.

The reasoning and search fields added to the test fixture align with ModelConfigHelper's extraction logic (reasoning.default → reasoning boolean, search.default → enableSearch, etc.). The first test at lines 98-102 validates these mappings correctly.


166-186: LGTM! Provider overlay behavior correctly validated.

This test confirms that provider-sourced configs dynamically overlay tool_call from the Provider DB on each read. Even though the stored config has functionCall: false, the overlay logic replaces it with the DB value (true initially, then false after the DB update at line 183). This ensures provider-managed configs stay synchronized with the Provider DB.


188-200: LGTM! User config precedence correctly validated.

This test confirms that user-defined configs (source defaults to 'user' when not specified) are not overlaid with Provider DB values. The user's functionCall: false is preserved even after the DB tool_call changes to true at line 197, ensuring explicit user overrides take precedence.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zerob13 zerob13 closed this Oct 13, 2025
@zerob13 zerob13 deleted the codex/add-update-logic-for-tool-call-settings branch January 6, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants