Skip to content

feat(provider): support reset/default subcommands to clear session override#8752

Open
Rat0323 wants to merge 4 commits into
AstrBotDevs:masterfrom
Rat0323:feat/provider-reset-command
Open

feat(provider): support reset/default subcommands to clear session override#8752
Rat0323 wants to merge 4 commits into
AstrBotDevs:masterfrom
Rat0323:feat/provider-reset-command

Conversation

@Rat0323

@Rat0323 Rat0323 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Description

Currently, when an administrator runs /provider <idx> in a session, the session-level provider override is persisted in the database under provider_perf_<type>.
However, there was no command or method to clear/delete this session-level preference once it was set. As a result, even if the administrator changes the default model or provider in the global WebUI configuration, the session remains locked/stuck on the old overridden provider.

This PR implements default, reset, and clear subcommands for:

  • /provider (e.g. /provider reset or /provider default)
  • /provider tts (e.g. /provider tts reset or /provider tts default)
  • /provider stt (e.g. /provider stt reset or /provider stt default)

When executed, it removes the session override preference using sp.session_remove(...) so that the session correctly falls back to the default configuration.

Proposed Changes

  • Change signature of idx2 in provider command to str | int | None to support string commands.
  • Check for subcommands ("default", "reset", "clear") for Chat, TTS, and STT providers.
  • Call sp.session_remove to delete the preference override keys.
  • Robustly parse provider index input as both string digits and integers.
  • Add unit tests for the newly added reset features in tests/unit/test_provider_commands.py.

Validation

All 3 new tests in tests/unit/test_provider_commands.py pass:

  • test_provider_reset_chat_completion
  • test_provider_reset_tts
  • test_provider_reset_stt

Summary by Sourcery

Add support for resetting session-level provider overrides back to the global defaults for chat, TTS, and STT providers.

New Features:

  • Allow /provider, /provider tts, and /provider stt commands to accept default/reset/clear subcommands to remove session-specific provider overrides.

Enhancements:

  • Relax provider command parameter types and parsing to handle both integer indices and string subcommands for provider selection.

Tests:

  • Add unit tests covering reset behavior for chat completion, TTS, and STT provider commands.

@sourcery-ai sourcery-ai Bot left a comment

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.

Hey - I've found 1 issue, and left some high level feedback:

  • The logic for parsing and validating provider indices (including the new default/reset/clear handling) is now duplicated across chat, TTS, and STT branches; consider extracting a small helper to centralize the digit conversion, bounds checking, and error messaging so future changes only need to be made in one place.
  • The three subcommand aliases ("default", "reset", "clear") are hardcoded in several separate conditionals; defining a shared constant or helper for the allowed reset commands would reduce the chance of them diverging between provider types over time.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic for parsing and validating provider indices (including the new `default/reset/clear` handling) is now duplicated across chat, TTS, and STT branches; consider extracting a small helper to centralize the digit conversion, bounds checking, and error messaging so future changes only need to be made in one place.
- The three subcommand aliases (`"default"`, `"reset"`, `"clear"`) are hardcoded in several separate conditionals; defining a shared constant or helper for the allowed reset commands would reduce the chance of them diverging between provider types over time.

## Individual Comments

### Comment 1
<location path="astrbot/builtin_stars/builtin_commands/commands/provider.py" line_range="195-194" />
<code_context>
                 )
                 return
-            if idx2 > len(self.context.get_all_tts_providers()) or idx2 < 1:
+            if idx2 in ("default", "reset", "clear"):
+                await sp.session_remove(
+                    umo,
+                    f"provider_perf_{ProviderType.TEXT_TO_SPEECH.value}",
+                )
+                event.set_result(
+                    MessageEventResult().message("✅ Successfully reset TTS provider to global default.")
+                )
+                return
+            try:
+                idx2_int = int(idx2)
</code_context>
<issue_to_address>
**suggestion:** Consider extracting the TTS/STT reset path into a shared helper to avoid divergence.

The TTS and STT reset branches both perform the same sequence: check for "default"/"reset"/"clear", call `session_remove` with a provider-specific key, then return a success message. A shared helper (e.g., `reset_provider(ProviderType, umo, success_message)`) would reduce duplication and keep their behavior consistent when you change the semantics or key format later.

Suggested implementation:

```python
            if await reset_provider(
                sp=sp,
                umo=umo,
                provider_type=ProviderType.TEXT_TO_SPEECH,
                idx_value=idx2,
                event=event,
                success_message="✅ Successfully reset TTS provider to global default.",
            ):
                return
            try:

```

1. Define the shared helper (either as a module-level coroutine or a private method on the same class) that encapsulates the reset logic used by both TTS and STT branches, for example:

```python
async def reset_provider(
    sp,
    umo,
    provider_type: ProviderType,
    idx_value: str | int | None,
    event,
    success_message: str,
) -> bool:
    if idx_value not in ("default", "reset", "clear"):
        return False

    await sp.session_remove(
        umo,
        f"provider_perf_{provider_type.value}",
    )
    event.set_result(
        MessageEventResult().message(success_message)
    )
    return True
```

2. Update the STT reset branch to use this helper in the same way as the TTS branch, for example:

```python
if await reset_provider(
    sp=sp,
    umo=umo,
    provider_type=ProviderType.SPEECH_TO_TEXT,
    idx_value=idx2,  # or the appropriate variable for STT’s index argument
    event=event,
    success_message="✅ Successfully reset STT provider to global default.",
):
    return
```

3. Ensure any type hints (e.g., `ProviderType`, `MessageEventResult`) and imports remain consistent with the rest of the file.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread astrbot/builtin_stars/builtin_commands/commands/provider.py

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds support for resetting Chat, TTS, and STT providers to their global defaults using "default", "reset", or "clear" subcommands, updates parameter type annotations, and introduces unit tests to verify this behavior. The feedback recommends refactoring the duplicated reset logic into a shared helper method and caching the retrieved provider lists locally to avoid redundant, inefficient method calls.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread astrbot/builtin_stars/builtin_commands/commands/provider.py
Comment on lines +211 to +216
if idx2_int > len(self.context.get_all_tts_providers()) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_tts_providers()[idx2 - 1]
provider = self.context.get_all_tts_providers()[idx2_int - 1]

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.

medium

Calling self.context.get_all_tts_providers() multiple times is inefficient and potentially unsafe if it returns a generator or a newly constructed sequence. Converting it to a list once and reusing it is more efficient and robust.

Suggested change
if idx2_int > len(self.context.get_all_tts_providers()) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_tts_providers()[idx2 - 1]
provider = self.context.get_all_tts_providers()[idx2_int - 1]
providers = list(self.context.get_all_tts_providers())
if idx2_int > len(providers) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = providers[idx2_int - 1]

Comment on lines +248 to +253
if idx2_int > len(self.context.get_all_stt_providers()) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_providers()[idx - 1]
provider = self.context.get_all_stt_providers()[idx2_int - 1]

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.

medium

Calling self.context.get_all_stt_providers() multiple times is inefficient and potentially unsafe if it returns a generator or a newly constructed sequence. Converting it to a list once and reusing it is more efficient and robust.

Suggested change
if idx2_int > len(self.context.get_all_stt_providers()) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_providers()[idx - 1]
provider = self.context.get_all_stt_providers()[idx2_int - 1]
providers = list(self.context.get_all_stt_providers())
if idx2_int > len(providers) or idx2_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = providers[idx2_int - 1]

Comment on lines +279 to +284
if idx_int > len(self.context.get_all_providers()) or idx_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_providers()[idx_int - 1]

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.

medium

Calling self.context.get_all_providers() multiple times is inefficient and potentially unsafe if it returns a generator or a newly constructed sequence. Converting it to a list once and reusing it is more efficient and robust.

Suggested change
if idx_int > len(self.context.get_all_providers()) or idx_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = self.context.get_all_providers()[idx_int - 1]
providers = list(self.context.get_all_providers())
if idx_int > len(providers) or idx_int < 1:
event.set_result(
MessageEventResult().message("❌ Invalid provider index.")
)
return
provider = providers[idx_int - 1]

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.

1 participant