Skip to content

feat(model): add top p generation setting#1680

Merged
zerob13 merged 2 commits into
devfrom
feat-topp
May 27, 2026
Merged

feat(model): add top p generation setting#1680
zerob13 merged 2 commits into
devfrom
feat-topp

Conversation

@zhangmo8
Copy link
Copy Markdown
Collaborator

@zhangmo8 zhangmo8 commented May 27, 2026

close #1165

image image

Summary by CodeRabbit

  • New Features
    • Added optional "Top P" (nucleus sampling) generation setting with UI controls in chat advanced settings and model config; persisted per-session and sent to the model only when set.
  • Validation
    • Top P validated between 0.1 and 1 (inputs clamp/step by 0.1).
  • Documentation
    • Spec, plan and task docs for feature and tooltip changes.
  • Localization
    • Added translations for Top P across supported locales.
  • Tests
    • Unit tests for persistence, validation and schema/migration behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

This pull request implements optional per-session topP (nucleus sampling) end-to-end: UI draft state and controls, shared Zod/type validation, nullable SQLite persistence (schema v29), runtime normalization/sanitization, conditional AI SDK passthrough for generate/stream, broad i18n coverage, and tests.

Changes

Top P Nucleus Sampling Feature

Layer / File(s) Summary
Feature documentation
docs/features/model-top-p-settings/plan.md, spec.md, tasks.md
Specification and implementation plan documenting topP requirements (0.1 ≤ topP ≤ 1), end-to-end data flow, persistence strategy, and test approach.
Shared validation contracts and types
src/shared/contracts/common.ts, src/shared/contracts/domainSchemas.ts, src/shared/types/agent-interface.d.ts, src/shared/types/presenters/legacy.presenters.d.ts, src/shared/utils/generationSettingsValidation.ts, src/types/i18n.d.ts
Zod schemas and TypeScript interfaces define topP as optional field constrained to [0.1, 1]; validation enforces finite-number check and range bounds with top_p_out_of_range error code.
SQLite persistence: schema version 29
src/main/presenter/sqlitePresenter/tables/deepchatSessions.ts, src/main/presenter/sqlitePresenter/schemaCatalog.ts
deepchat_sessions table adds nullable top_p REAL in schema v29; migration, creation DDL, and read/write/update paths serialize topP in session generation settings; latest version bumped to 29.
Main process: model config builder
src/main/presenter/configPresenter/modelConfig.ts
ModelConfigHelper includes topP: undefined when constructing initial/fallback/merged model configs.
Main process: generation settings lifecycle
src/main/presenter/agentRuntimePresenter/index.ts
normalizeTopP helper validates/clamps topP and integrates it into default/resolved generation settings, persisted patch/replacement builders, and sanitization during updates.
Main process: AI SDK integration
src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts
runAiSdkGenerateText and runAiSdkCoreStream conditionally pass normalizedModelConfig.topP into generateText/streamText request bodies when defined.
Renderer: chat UI control
src/renderer/src/components/chat/ChatStatusBar.vue
Conditional TopP numeric control (step/min/max constants, increment/decrement, input commit/clear, tooltip provider), draft/error state management, normalization/persist patching, and preserved/cleared behavior during model switching.
Renderer: model config dialog
src/renderer/src/components/settings/ModelConfigDialog.vue
TopP override input backed by topPDraft, clamped/validated to [0.1, 1], synced on load/reset, and conditionally persisted into saved config.
Renderer: draft state management
src/renderer/src/stores/ui/draft.ts, src/renderer/src/pages/NewThreadPage.vue
Draft store adds topP ref; conversion/update/reset flows include topP; NewThreadPage reset clears draft topP.
Internationalization
src/renderer/src/i18n/{da-DK,de-DE,en-US,es-ES,fa-IR,fr-FR,he-IL,id-ID,it-IT,ja-JP,ko-KR,ms-MY,pl-PL,pt-BR,ru-RU,tr-TR,vi-VN,zh-CN,zh-HK,zh-TW}/{chat,settings}.json, src/types/i18n.d.ts
TopP UI strings (label, description) and validation message topPRange added/updated across many locale files and i18n type definitions.
Tests
test/main/presenter/sqlitePresenter/deepchatSessionsTable.test.ts, test/main/shared/generationSettingsValidation.test.ts, test/renderer/stores/draft.test.ts
Tests added/updated for validation behavior, migration SQL inclusion and version bump (→29), persisted row shape, and draft-store override/clear semantics.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • ThinkInAIXYZ/deepchat#516: Foundation for custom model-parameter editing in ModelConfigDialog, extended here to add topP input/validation/persistence.
  • ThinkInAIXYZ/deepchat#1360: Related changes to deepchatSessions schema/versioning and migration logic used by this PR.
  • ThinkInAIXYZ/deepchat#1490: Overlaps on deepchatSessions schema/versioning and migration handling; likely related to the schema bump and recovery migrations.

Suggested reviewers

  • zerob13

🐰 I hopped through code and tests so spry,
Top P tucked in drafts and DB, oh my—
From UI tip to model's call,
Across locales big and small,
Rabbit nods: small step, big supply.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(model): add top p generation setting' clearly summarizes the main change—adding topP as a new generation setting for the model configuration system.
Linked Issues check ✅ Passed The PR fully implements issue #1165's requirement to add an option to adjust the model topP parameter, including UI controls [ChatStatusBar.vue, ModelConfigDialog.vue], persistence [deepchatSessions.ts], runtime wiring [agentRuntimePresenter, runtime.ts], and comprehensive i18n support across 23 languages.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the topP feature: documentation planning/spec, schema/type updates, persistence layer, runtime integration, UI components, and i18n additions. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-topp

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 18

🧹 Nitpick comments (2)
test/main/presenter/sqlitePresenter/deepchatSessionsTable.test.ts (1)

133-135: ⚡ Quick win

Remove duplicate assertion.

Lines 111-120 already verify the complete v23 migration SQL with exact string equality, which confirms the top_p REAL column is included. The toContain check here is redundant.

♻️ Proposed fix
     expect(table.getMigrationSQL(29)).toBe('ALTER TABLE deepchat_sessions ADD COLUMN top_p REAL;')
-
-    expect(table.getMigrationSQL(23)).toContain(
-      'ALTER TABLE deepchat_sessions ADD COLUMN top_p REAL;'
-    )
   })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/main/presenter/sqlitePresenter/deepchatSessionsTable.test.ts` around
lines 133 - 135, Remove the redundant assertion that checks
table.getMigrationSQL(23) contains 'ALTER TABLE deepchat_sessions ADD COLUMN
top_p REAL;' — since the v23 migration SQL is already validated with an exact
equality assertion earlier (lines 111–120), delete the expect(...toContain(...))
line referencing table.getMigrationSQL(23) to avoid duplication; keep the
comprehensive equality check and any surrounding setup/asserts (i.e., the
table.getMigrationSQL(23) call used in the earlier exact-match assertion).
test/renderer/stores/draft.test.ts (1)

6-9: ⚡ Quick win

Remove unnecessary mock that delegates to actual implementation.

The mock delegates directly to the actual Pinia module without modification, making it redundant. Remove the vi.doMock call entirely.

♻️ Proposed fix
   beforeEach(() => {
     vi.resetModules()
-    vi.doMock('pinia', async () => {
-      const actual = await vi.importActual<typeof import('pinia')>('pinia')
-      return actual
-    })
   })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/renderer/stores/draft.test.ts` around lines 6 - 9, The test contains a
redundant mock: remove the vi.doMock call that simply re-exports the actual
Pinia module. Delete the vi.doMock('pinia', ...) block in
test/renderer/stores/draft.test.ts so the real Pinia import is used directly (no
replacement of import behavior in the test), ensuring any setup/uses of Pinia in
the test continue to import the real module.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/presenter/agentRuntimePresenter/index.ts`:
- Around line 3628-3635: The current update path deletes next.topP when
normalizeTopP(patch.topP) returns undefined, which silently resets a valid
session value for inputs like 2 or 'abc'; change the logic in the block handling
patch.topP (where normalizeTopP is called) so that if normalizeTopP returns a
finite value you assign next.topP = normalizedTopP, but if it returns undefined
you do nothing (leave next.topP unchanged) instead of deleting it; reference the
normalizeTopP function, the patch object and the next.topP assignment to locate
and update the code.

In `@src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts`:
- Around line 1182-1183: The spread currently emits normalizedModelConfig.topP
whenever it's not undefined, which allows invalid values (0, NaN, >1) to reach
the AI SDK; update the emission to only include topP when it's a finite number
inside the valid probability range (e.g.,
Number.isFinite(normalizedModelConfig.topP) && normalizedModelConfig.topP > 0 &&
normalizedModelConfig.topP <= 1) so the object spread uses a guarded expression
like ...(validTopP ? { topP: normalizedModelConfig.topP } : {}); apply the same
guarded check for all occurrences that reference normalizedModelConfig.topP in
this file (the spreads around the current diff and the other spots you noted).

In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 1089-1090: The UI clamps topP using TOP_P_MIN = 0.01 which is
stricter than the validation range (0,1], causing decrement/button logic to jump
small valid values (e.g., 0.005) up to 0.01; change the clamp minimum to 0 (or
to a value matching validation) and keep the button step at 0.01 if desired so
manual inputs <0.01 remain allowed — update all occurrences (TOP_P_MIN/TOP_P_MAX
and any clamp calls referencing them, plus related UI step props) and ensure
topP is still validated as >0 and <=1 in the input/submit validation paths
(e.g., where topP is used/validated and where the decrement/increment handlers
reference TOP_P_MIN).

In `@src/renderer/src/i18n/fa-IR/chat.json`:
- Line 170: Replace the English strings in the Persian locale JSON for the keys
"topPDescription" and the key at line 188 with Persian translations so the fa-IR
file is fully localized; open src/renderer/src/i18n/fa-IR/chat.json, locate the
"topPDescription" entry and the other untranslated entry at line 188, and update
their values to appropriate Persian text while preserving the JSON key names and
punctuation.

In `@src/renderer/src/i18n/fa-IR/settings.json`:
- Around line 627-630: The fa-IR locale file has English text for the recently
added keys; update the fa-IR entries for "topP.description" and
"validation.topPRange" to Persian translations (and also fix the duplicate at
the other occurrence around the 655 area) so the UI is fully localized; locate
the "topP" object and the "validation" object in
src/renderer/src/i18n/fa-IR/settings.json and replace the English strings with
proper Persian strings for the keys topP.description and validation.topPRange.

In `@src/renderer/src/i18n/he-IL/chat.json`:
- Around line 169-170: Translate the English strings for the topP UI into Hebrew
by replacing the current values for "topPDescription" and "validation.topPRange"
in the he-IL locale file so the advanced settings panel is fully localized;
specifically update the "topPDescription" value (currently "Nucleus sampling;
leave blank to use provider default") and the "validation.topPRange" value to
their appropriate Hebrew translations while keeping the keys "topP",
"topPDescription", and "validation.topPRange" unchanged.

In `@src/renderer/src/i18n/he-IL/settings.json`:
- Around line 627-630: Translate the English strings for the newly added keys
into Hebrew: replace topP.description ("Controls nucleus sampling. Leave blank
to use the model or provider default.") with an appropriate Hebrew translation,
and update validation.topPRange (the validation message around allowed Top P
values) to Hebrew as well; there are two occurrences (the topP block and the
validation.topPRange entries referenced around the later lines), so update both
topP.description and validation.topPRange to their Hebrew equivalents while
preserving the keys and punctuation.

In `@src/renderer/src/i18n/id-ID/chat.json`:
- Around line 180-181: The messages for the keys topP, topPDescription, and
validation.topPRange are still in English; update their values to Indonesian so
the advanced settings UI is fully localized: translate the string for "topP"
(label), "topPDescription" (description: "Nucleus sampling; leave blank to use
provider default") and the validation message under validation.topPRange into
natural Indonesian, preserving the existing key names (topP, topPDescription,
validation.topPRange) and quotation/JSON formatting.

In `@src/renderer/src/i18n/id-ID/settings.json`:
- Around line 626-629: The new "topP" entries in
src/renderer/src/i18n/id-ID/settings.json still have English text; update the
"topP.description" value to an Indonesian translation of "Controls nucleus
sampling. Leave blank to use the model or provider default." and also translate
the "topPRange" validation message (the second occurrence around lines 730-731)
into Indonesian so both the label/description and the validation string are
localized consistently; locate the "topP" and "topPRange" keys in the file and
replace their English strings with appropriate Indonesian equivalents.

In `@src/renderer/src/i18n/ms-MY/chat.json`:
- Around line 180-181: The ms-MY locale file still contains English for the
"topP" and "topPDescription" keys (and the other instance around line 199);
replace those English values with proper Malay translations so the UI is fully
localized—update the "topP" value to a short Malay label (e.g., "Top P" -> "Top
P" or a Malay equivalent) and change "topPDescription" to a Malay sentence
conveying "Nucleus sampling; leave blank to use provider default" (e.g.,
"Pensampelan teras; biarkan kosong untuk menggunakan nilai lalai penyedia"), and
do the same for the duplicate key instance.

In `@src/renderer/src/i18n/ms-MY/settings.json`:
- Around line 626-629: The "topP" entry in the ms-MY settings JSON has English
text; replace its label/description with Malay and ensure the duplicate "topP"
occurrence later uses the same translation — specifically update the "topP"
key's "label" to "Top P" (retain the technical name) and the "description" to a
Malay string such as "Mengawal pensampelan teras (nucleus). Biarkan kosong untuk
menggunakan nilai lalai model atau penyedia." and apply the identical changes to
the other "topP" instance so both entries are consistent.

In `@src/renderer/src/i18n/pl-PL/chat.json`:
- Around line 180-181: The Polish locale has untranslated strings for the
advanced-settings keys "topP" and "topPDescription" (and the related validation
message referenced at the second occurrence around "topP" key); update the
values for those JSON keys in pl-PL chat.json to their proper Polish
translations while keeping the keys unchanged and preserving punctuation and
capitalization so the UI and validation messages display fully in Polish.

In `@src/renderer/src/i18n/pl-PL/settings.json`:
- Around line 690-693: The Polish locale currently contains English text for the
new Top P UI strings: translate the entries under the "topP" key (label and
description) into Polish and update any adjacent/new keys introduced near "topP"
(the other strings added around the same area) so they are fully localized;
locate the "topP" object in src/renderer/src/i18n/pl-PL/settings.json and
replace the English values with their Polish equivalents, making sure the keys
stay unchanged.

In `@src/renderer/src/i18n/ru-RU/chat.json`:
- Around line 169-170: The value for the i18n key "topPDescription" currently
mixes English and Russian; replace the mixed-language string with a fully
Russian translation for consistency (update the "topPDescription" entry to a
complete Russian sentence describing nucleus sampling and the default provider
behavior, keeping the "topP" key unchanged).

In `@src/renderer/src/i18n/vi-VN/chat.json`:
- Line 181: The vi-VN locale JSON still contains English help strings for keys
like "topPDescription" (and the other untranslated key around line 199); replace
those English values with proper Vietnamese translations so the UI is fully
localized—locate the keys "topPDescription" and the untranslated key near line
199 in src/renderer/src/i18n/vi-VN/chat.json and update their string values to
Vietnamese equivalents that convey the same meaning as the originals.

In `@src/renderer/src/i18n/vi-VN/settings.json`:
- Around line 690-693: The vi-VN locale has English text for topP.description
and validation.topPRange; locate the "topP" entry (key "topP" with
"label"/"description") and the "validation.topPRange" string in the vi-VN
settings.json and replace the English phrases with Vietnamese equivalents (e.g.,
description -> "Điều khiển nucleus sampling. Để trống để sử dụng mặc định của mô
hình hoặc nhà cung cấp." and validation.topPRange -> a Vietnamese range
validation message like "Vui lòng nhập một giá trị giữa 0 và 1."), and ensure
the same change is applied to the other occurrence mentioned
(validation.topPRange elsewhere in the file).

In `@src/renderer/src/i18n/zh-HK/chat.json`:
- Around line 177-178: The zh-HK locale entries for the sampling parameter
strings are still in English; update the "topP", "topPDescription", and
"topPRange" keys in chat.json to proper Cantonese (zh-HK) localized copy
(replace "Top P", "Nucleus sampling; leave blank to use provider default", and
the English range text with their zh-HK equivalents), ensuring the translated
strings are used for both occurrences mentioned (including the second occurrence
referenced at lines 196); keep the key names unchanged so existing code (e.g.,
lookups for "topP", "topPDescription", "topPRange") continues to work.

In `@src/renderer/src/i18n/zh-HK/settings.json`:
- Around line 627-630: The zh-HK locale still has the new topP strings in
English; update the "topP" entry's "label" and "description" (and the related
range/validation message for the same topP key elsewhere) to Traditional
Chinese, replacing "Top P", "Controls nucleus sampling. Leave blank to use the
model or provider default." and any English range/validation text with
appropriate zh-HK translations so both occurrences of the topP localization are
fully translated.

---

Nitpick comments:
In `@test/main/presenter/sqlitePresenter/deepchatSessionsTable.test.ts`:
- Around line 133-135: Remove the redundant assertion that checks
table.getMigrationSQL(23) contains 'ALTER TABLE deepchat_sessions ADD COLUMN
top_p REAL;' — since the v23 migration SQL is already validated with an exact
equality assertion earlier (lines 111–120), delete the expect(...toContain(...))
line referencing table.getMigrationSQL(23) to avoid duplication; keep the
comprehensive equality check and any surrounding setup/asserts (i.e., the
table.getMigrationSQL(23) call used in the earlier exact-match assertion).

In `@test/renderer/stores/draft.test.ts`:
- Around line 6-9: The test contains a redundant mock: remove the vi.doMock call
that simply re-exports the actual Pinia module. Delete the vi.doMock('pinia',
...) block in test/renderer/stores/draft.test.ts so the real Pinia import is
used directly (no replacement of import behavior in the test), ensuring any
setup/uses of Pinia in the test continue to import the real module.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac84c48c-bf2d-47ff-9201-4218d37f652e

📥 Commits

Reviewing files that changed from the base of the PR and between 9d81d6f and c90e59b.

📒 Files selected for processing (61)
  • docs/features/model-top-p-settings/plan.md
  • docs/features/model-top-p-settings/spec.md
  • docs/features/model-top-p-settings/tasks.md
  • src/main/presenter/agentRuntimePresenter/index.ts
  • src/main/presenter/configPresenter/modelConfig.ts
  • src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts
  • src/main/presenter/sqlitePresenter/schemaCatalog.ts
  • src/main/presenter/sqlitePresenter/tables/deepchatSessions.ts
  • src/renderer/src/components/chat/ChatStatusBar.vue
  • src/renderer/src/components/settings/ModelConfigDialog.vue
  • src/renderer/src/i18n/da-DK/chat.json
  • src/renderer/src/i18n/da-DK/settings.json
  • src/renderer/src/i18n/de-DE/chat.json
  • src/renderer/src/i18n/de-DE/settings.json
  • src/renderer/src/i18n/en-US/chat.json
  • src/renderer/src/i18n/en-US/settings.json
  • src/renderer/src/i18n/es-ES/chat.json
  • src/renderer/src/i18n/es-ES/settings.json
  • src/renderer/src/i18n/fa-IR/chat.json
  • src/renderer/src/i18n/fa-IR/settings.json
  • src/renderer/src/i18n/fr-FR/chat.json
  • src/renderer/src/i18n/fr-FR/settings.json
  • src/renderer/src/i18n/he-IL/chat.json
  • src/renderer/src/i18n/he-IL/settings.json
  • src/renderer/src/i18n/id-ID/chat.json
  • src/renderer/src/i18n/id-ID/settings.json
  • src/renderer/src/i18n/it-IT/chat.json
  • src/renderer/src/i18n/it-IT/settings.json
  • src/renderer/src/i18n/ja-JP/chat.json
  • src/renderer/src/i18n/ja-JP/settings.json
  • src/renderer/src/i18n/ko-KR/chat.json
  • src/renderer/src/i18n/ko-KR/settings.json
  • src/renderer/src/i18n/ms-MY/chat.json
  • src/renderer/src/i18n/ms-MY/settings.json
  • src/renderer/src/i18n/pl-PL/chat.json
  • src/renderer/src/i18n/pl-PL/settings.json
  • src/renderer/src/i18n/pt-BR/chat.json
  • src/renderer/src/i18n/pt-BR/settings.json
  • src/renderer/src/i18n/ru-RU/chat.json
  • src/renderer/src/i18n/ru-RU/settings.json
  • src/renderer/src/i18n/tr-TR/chat.json
  • src/renderer/src/i18n/tr-TR/settings.json
  • src/renderer/src/i18n/vi-VN/chat.json
  • src/renderer/src/i18n/vi-VN/settings.json
  • src/renderer/src/i18n/zh-CN/chat.json
  • src/renderer/src/i18n/zh-CN/settings.json
  • src/renderer/src/i18n/zh-HK/chat.json
  • src/renderer/src/i18n/zh-HK/settings.json
  • src/renderer/src/i18n/zh-TW/chat.json
  • src/renderer/src/i18n/zh-TW/settings.json
  • src/renderer/src/pages/NewThreadPage.vue
  • src/renderer/src/stores/ui/draft.ts
  • src/shared/contracts/common.ts
  • src/shared/contracts/domainSchemas.ts
  • src/shared/types/agent-interface.d.ts
  • src/shared/types/presenters/legacy.presenters.d.ts
  • src/shared/utils/generationSettingsValidation.ts
  • src/types/i18n.d.ts
  • test/main/presenter/sqlitePresenter/deepchatSessionsTable.test.ts
  • test/main/shared/generationSettingsValidation.test.ts
  • test/renderer/stores/draft.test.ts

Comment thread src/main/presenter/agentRuntimePresenter/index.ts
Comment thread src/main/presenter/llmProviderPresenter/aiSdk/runtime.ts
Comment thread src/renderer/src/components/chat/ChatStatusBar.vue Outdated
Comment thread src/renderer/src/i18n/fa-IR/chat.json
Comment thread src/renderer/src/i18n/fa-IR/settings.json
Comment thread src/renderer/src/i18n/ru-RU/chat.json
Comment thread src/renderer/src/i18n/vi-VN/chat.json
Comment thread src/renderer/src/i18n/vi-VN/settings.json
Comment thread src/renderer/src/i18n/zh-HK/chat.json
Comment thread src/renderer/src/i18n/zh-HK/settings.json
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/renderer/src/components/chat/ChatStatusBar.vue (1)

1115-1117: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Top P floor at 0.1 now silently coerces/drops lower values.

Lines 1116, 2881, and 2915-2919 enforce topP >= 0.1 and auto-clamp on commit. This can unexpectedly rewrite existing or manually entered low values instead of surfacing a validation error, and it narrows controllability for models that support finer/low nucleus sampling.

Suggested adjustment
-const TOP_P_STEP = 0.1
-const TOP_P_MIN = 0.1
+const TOP_P_STEP = 0.01
+const TOP_P_MIN = Number.EPSILON
@@
 function normalizeTopP(value: unknown): number | undefined {
   const numeric = parseFiniteNumericValue(value)
-  return numeric !== undefined && numeric >= 0.1 && numeric <= 1 ? numeric : undefined
+  return numeric !== undefined && numeric > 0 && numeric <= 1 ? numeric : undefined
 }
@@
-  const draftNum = parseFiniteNumericValue(numericInputDrafts.value.topP)
-  if (draftNum !== undefined) {
-    if (draftNum < TOP_P_MIN) {
-      numericInputDrafts.value.topP = String(TOP_P_MIN)
-    } else if (draftNum > TOP_P_MAX) {
-      numericInputDrafts.value.topP = String(TOP_P_MAX)
-    }
-  }
-
   const next = commitNumericField('topP', numericInputDrafts.value.topP)

Also applies to: 2879-2882, 2913-2920

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderer/src/components/chat/ChatStatusBar.vue` around lines 1115 - 1117,
The code is silently coercing low nucleus-sampling values because TOP_P_MIN is
set to 0.1 and clamp logic auto-writes on commit; change TOP_P_MIN from 0.1 to 0
(or remove the hard floor) and stop auto-clamping in the commit/submit handler
(the method that uses TOP_P_MIN/TOP_P_MAX and performs the clamp) — instead add
explicit validation that rejects values below model-supported min and surfaces a
UI validation error/message so manual low values are not overwritten; keep
TOP_P_STEP as-is unless a finer step is required and ensure any clamp calls
reference the new TOP_P_MIN or are only applied as an upper bound, not silently
on commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/issues/chat-top-p-tooltip/plan.md`:
- Line 17: Update the plan text to reflect that Top P clamp behavior was changed
in ChatStatusBar.vue: state that the minimum Top P is now 0.1 (previously
lower), that commit-time clamping is applied (values are clamped when
committing/ saving), and any persistence/tests should expect the tightened
min/clamp behavior rather than "unchanged"; reference ChatStatusBar.vue and the
Top P min/clamp change so reviewers/testers use the correct assumptions.

---

Duplicate comments:
In `@src/renderer/src/components/chat/ChatStatusBar.vue`:
- Around line 1115-1117: The code is silently coercing low nucleus-sampling
values because TOP_P_MIN is set to 0.1 and clamp logic auto-writes on commit;
change TOP_P_MIN from 0.1 to 0 (or remove the hard floor) and stop auto-clamping
in the commit/submit handler (the method that uses TOP_P_MIN/TOP_P_MAX and
performs the clamp) — instead add explicit validation that rejects values below
model-supported min and surfaces a UI validation error/message so manual low
values are not overwritten; keep TOP_P_STEP as-is unless a finer step is
required and ensure any clamp calls reference the new TOP_P_MIN or are only
applied as an upper bound, not silently on commit.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f8bb7365-ea03-4f44-ad4d-2d070d686f6f

📥 Commits

Reviewing files that changed from the base of the PR and between c90e59b and 52360ba.

📒 Files selected for processing (13)
  • docs/features/model-top-p-settings/plan.md
  • docs/features/model-top-p-settings/spec.md
  • docs/features/model-top-p-settings/tasks.md
  • docs/issues/chat-top-p-tooltip/plan.md
  • docs/issues/chat-top-p-tooltip/spec.md
  • docs/issues/chat-top-p-tooltip/tasks.md
  • src/main/presenter/agentRuntimePresenter/index.ts
  • src/renderer/src/components/chat/ChatStatusBar.vue
  • src/renderer/src/components/settings/ModelConfigDialog.vue
  • src/shared/contracts/common.ts
  • src/shared/contracts/domainSchemas.ts
  • src/shared/utils/generationSettingsValidation.ts
  • test/main/shared/generationSettingsValidation.test.ts
✅ Files skipped from review due to trivial changes (4)
  • docs/issues/chat-top-p-tooltip/tasks.md
  • docs/issues/chat-top-p-tooltip/spec.md
  • docs/features/model-top-p-settings/tasks.md
  • docs/features/model-top-p-settings/spec.md
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/shared/contracts/common.ts
  • docs/features/model-top-p-settings/plan.md
  • test/main/shared/generationSettingsValidation.test.ts
  • src/shared/contracts/domainSchemas.ts
  • src/shared/utils/generationSettingsValidation.ts
  • src/main/presenter/agentRuntimePresenter/index.ts
  • src/renderer/src/components/settings/ModelConfigDialog.vue


## Compatibility

No persisted configuration changes. The chat settings Top P clamp behavior remains unchanged.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Plan text is stale relative to this PR’s Top P behavior.

Line 17 says clamp behavior is unchanged, but this PR also tightens Top P min/clamp behavior in ChatStatusBar.vue (e.g., min set to 0.1 and commit-time clamping). Please update the plan so reviewers/testers aren’t working from incorrect assumptions.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/issues/chat-top-p-tooltip/plan.md` at line 17, Update the plan text to
reflect that Top P clamp behavior was changed in ChatStatusBar.vue: state that
the minimum Top P is now 0.1 (previously lower), that commit-time clamping is
applied (values are clamped when committing/ saving), and any persistence/tests
should expect the tightened min/clamp behavior rather than "unchanged";
reference ChatStatusBar.vue and the Top P min/clamp change so reviewers/testers
use the correct assumptions.

@zerob13 zerob13 merged commit 8e8550b into dev May 27, 2026
3 checks passed
@zhangmo8 zhangmo8 deleted the feat-topp branch May 27, 2026 07:14
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.

[Feature] 支持调整模型 top_p

2 participants