remove(llm): retire think_enabled setting entirely
Two-in-one cleanup motivated by the chat hang in dev 2026-05-22.
The crash root cause from the guarded-task traceback:
UnboundLocalError: cannot access local variable 'get_setting'
where it is not associated with a value
File generation_task.py:257, in run_generation
think = (await get_setting(user_id, 'think_enabled', 'false'))...
generation_task.py imports get_setting at module top, but a later
'if voice_mode: from ... import get_setting' block scopes it as a
function-local. When voice_mode=False the local import never runs,
but Python had already flagged get_setting as local for the entire
body — the think_enabled read at line 257 hit UnboundLocalError.
The line itself was dead-weight anyway. With the conversation+curator
architecture: chat ships tools=[] (think on a no-tools pass is pure
latency cost; nothing for the model to reason ABOUT in tool-call
terms), and the curator hardcodes think=False already. The user
setting was a holdover from before the architecture pivot. Removing
it entirely is cleaner than fixing the scoping bug to preserve a
toggle nobody should be using:
- generation_task.py: think hardcoded False. Removed the get_setting
call (which fixes the UnboundLocalError as a side effect).
- SettingsView.vue: dropped the Enable model thinking checkbox, the
thinkEnabled / savingThinkEnabled refs, the saveThinkEnabled
function, and the think_enabled load step.
- Migration 0050: DELETE FROM settings WHERE key='think_enabled'
to clean up any stored rows.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,27 @@
|
||||
"""drop stored think_enabled rows — setting was removed
|
||||
|
||||
Revision ID: 0050
|
||||
Revises: 0049
|
||||
Create Date: 2026-05-23
|
||||
|
||||
The `think_enabled` user setting was retired with the chat+curator
|
||||
architecture: chat has tools=[] and curator hardcodes think=False, so
|
||||
the toggle was dead weight. Any rows already in `settings` for that
|
||||
key are now unread by the app; this migration clears them.
|
||||
"""
|
||||
from alembic import op
|
||||
|
||||
|
||||
revision = "0050"
|
||||
down_revision = "0049"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.execute("DELETE FROM settings WHERE key = 'think_enabled'")
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
# No-op. The setting is dead; restoring NULL rows wouldn't help.
|
||||
pass
|
||||
@@ -20,8 +20,6 @@ const savingTimezone = ref(false);
|
||||
const timezoneSaved = ref(false);
|
||||
const autoConsolidateTasks = ref(true);
|
||||
const savingAutoConsolidate = ref(false);
|
||||
const thinkEnabled = ref(false);
|
||||
const savingThinkEnabled = ref(false);
|
||||
|
||||
async function saveAutoConsolidate() {
|
||||
savingAutoConsolidate.value = true;
|
||||
@@ -36,18 +34,10 @@ async function saveAutoConsolidate() {
|
||||
}
|
||||
}
|
||||
|
||||
async function saveThinkEnabled() {
|
||||
savingThinkEnabled.value = true;
|
||||
try {
|
||||
await apiPut('/api/settings', {
|
||||
think_enabled: thinkEnabled.value ? "true" : "false",
|
||||
});
|
||||
} catch {
|
||||
toastStore.show('Failed to save setting', 'error');
|
||||
} finally {
|
||||
savingThinkEnabled.value = false;
|
||||
}
|
||||
}
|
||||
// think_enabled setting removed 2026-05-23. The chat+curator architecture
|
||||
// has tools=[] on the chat model; think on a no-tools conversational pass
|
||||
// is pure latency cost. See generation_task.py:run_generation comment for
|
||||
// the full reasoning.
|
||||
|
||||
function detectTimezone() {
|
||||
userTimezone.value = Intl.DateTimeFormat().resolvedOptions().timeZone;
|
||||
@@ -826,9 +816,6 @@ onMounted(async () => {
|
||||
userTimezone.value = allSettings.user_timezone ?? "";
|
||||
// Default true if unset; explicit "false" disables auto-consolidation.
|
||||
autoConsolidateTasks.value = (allSettings.auto_consolidate_tasks ?? "true") !== "false";
|
||||
// Default false: think mode is opt-in. Bench showed it costs 1-2 min/turn
|
||||
// for unclear benefit; qwen3 users can re-enable if tool calls regress.
|
||||
thinkEnabled.value = (allSettings.think_enabled ?? "false") === "true";
|
||||
chatRetentionDays.value = allSettings.chat_retention_days !== undefined
|
||||
? Number(allSettings.chat_retention_days)
|
||||
: 90;
|
||||
@@ -1569,22 +1556,6 @@ function formatUserDate(iso: string): string {
|
||||
</span>
|
||||
</p>
|
||||
</div>
|
||||
<div class="field">
|
||||
<label class="checkbox-label">
|
||||
<input
|
||||
type="checkbox"
|
||||
v-model="thinkEnabled"
|
||||
:disabled="savingThinkEnabled"
|
||||
@change="saveThinkEnabled"
|
||||
/>
|
||||
Enable model thinking
|
||||
</label>
|
||||
<p class="field-hint">
|
||||
When on, the chat model runs in reasoning mode before responding.
|
||||
Default off — bench data showed thinking adds 1–2 minutes per turn for unclear quality benefit.
|
||||
Turn on for qwen3-family models if tool-call reliability drops without it.
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="actions">
|
||||
<button class="btn-save" @click="saveAssistant" :disabled="saving">
|
||||
|
||||
@@ -246,15 +246,17 @@ async def run_generation(
|
||||
# Emit context event
|
||||
buf.append_event("context", {"context": context_meta})
|
||||
|
||||
# Think mode is per-user-setting (default off). Historically forced on for
|
||||
# qwen3 because content-gated thinking (87fcaa6) exposed silent-generation
|
||||
# failures on short tool-intent prompts. After moving to a model-family
|
||||
# decoupled architecture, the bench data (May 2026) showed think costs
|
||||
# 1-2 min/turn for unclear quality benefit; default off, opt in via
|
||||
# the Settings UI. The generation_tool_log captures per-turn outcomes
|
||||
# so reliability regressions surface empirically.
|
||||
# Think mode is hardcoded off (2026-05-23). Historical context:
|
||||
# originally forced on for qwen3's combined think+tools template;
|
||||
# then made user-configurable when we decoupled the architecture;
|
||||
# now removed entirely because in the chat+curator world there's no
|
||||
# reason for the chat model to think. Chat has tools=[] — it just
|
||||
# talks, and think on a no-tools conversational pass is pure
|
||||
# latency cost (the May 2026 bench measured 1-2 min/turn for
|
||||
# unclear quality benefit). The curator (services/curator.py) also
|
||||
# already hardcodes think=False for its own reasons.
|
||||
think_requested = think
|
||||
think = (await get_setting(user_id, "think_enabled", "false")).lower() == "true"
|
||||
think = False
|
||||
|
||||
t_start = time.monotonic()
|
||||
timing: dict = {
|
||||
|
||||
Reference in New Issue
Block a user