Skip to content

Enhance model selection and add footer navigation instructions#1271

Merged
imguoguo merged 6 commits intosipeed:mainfrom
taorye:main
Mar 9, 2026
Merged

Enhance model selection and add footer navigation instructions#1271
imguoguo merged 6 commits intosipeed:mainfrom
taorye:main

Conversation

@taorye
Copy link
Collaborator

@taorye taorye commented Mar 9, 2026

📝 Description

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

  • Reference URL:
  • Reasoning:

🧪 Test Environment

  • Hardware: PC(x86_64)
  • OS: Arch
  • Model/Provider: openai/gpt-5.4
  • Channels: Telegram

📸 Evidence (Optional)

Click to view Logs/Screenshots image

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

Copilot AI review requested due to automatic review settings March 9, 2026 10:13
@taorye taorye added the type: enhancement New feature or request label Mar 9, 2026
@CLAassistant
Copy link

CLAassistant commented Mar 9, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the launcher TUI to improve navigation clarity (via a persistent footer) and refines model/channel selection behavior in the configuration menus.

Changes:

  • Adds a footer bar with keyboard navigation hints to the TUI layout.
  • Adjusts model menu behavior (row indexing, add-model entry placement, delete confirmation) and adds model-name validation.
  • Updates the main menu channel entry to show an enabled/total channel count.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
cmd/picoclaw-launcher-tui/internal/ui/style.go Adds a centered footer TextView with navigation instructions.
cmd/picoclaw-launcher-tui/internal/ui/app.go Integrates the footer into the root layout and shows enabled/total channel counts on the main menu.
cmd/picoclaw-launcher-tui/internal/ui/model.go Reworks model menu item ordering/indexing, adds add-model naming helper, validates model-name edits, and adds a delete confirmation modal.
cmd/picoclaw-launcher-tui/internal/ui/channel.go Removes the “Back” item from the channel menu list (relying on Esc navigation).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +133 to +149
func (s *appState) countChannels() (enabled int, total int) {
c := s.config.Channels
entries := []bool{
c.Telegram.Enabled,
c.Discord.Enabled,
c.QQ.Enabled,
c.MaixCam.Enabled,
c.WhatsApp.Enabled,
c.Feishu.Enabled,
c.DingTalk.Enabled,
c.Slack.Enabled,
c.Matrix.Enabled,
c.LINE.Enabled,
c.OneBot.Enabled,
c.WeCom.Enabled,
c.WeComApp.Enabled,
}
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

countChannels hard-codes the same enabled-channel list that already exists in hasEnabledChannel(). This duplication can easily drift when new channels are added/renamed. Consider deriving both enabled/total from a single shared slice/helper (e.g., have hasEnabledChannel call countChannels, or introduce a method returning the bools).

Copilot uses AI. Check for mistakes.
Comment on lines +102 to +105
if s.modelNameExists(value, index) {
s.showMessage("Duplicate model name", fmt.Sprintf("Model Name '%s' already exists", value))
return
}
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The new “duplicate model name” validation blocks having multiple model_list entries with the same model_name. In this repo, duplicates are explicitly allowed for load balancing (see Config.ValidateModelList / tests), so this change removes a supported configuration capability. Consider removing this check or turning it into a warning/UX hint rather than a hard error.

Suggested change
if s.modelNameExists(value, index) {
s.showMessage("Duplicate model name", fmt.Sprintf("Model Name '%s' already exists", value))
return
}
// Allow duplicate model names for load balancing; duplicates are validated elsewhere.

Copilot uses AI. Check for mistakes.
},
})
}
// Add model entry appended at the end so the models map to rows 1..N
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

This comment says models map to rows 1..N, but with the Back item removed the model rows are now 0..N-1 (and the Add Model row is at N). Update the comment to match the actual indexing to avoid future off-by-one regressions.

Suggested change
// Add model entry appended at the end so the models map to rows 1..N
// Add model entry appended at the end so the models map to rows 0..N-1 (and Add model is at row N)

Copilot uses AI. Check for mistakes.
taorye and others added 2 commits March 9, 2026 18:29
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 9, 2026 10:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to +57
Action: func() {
newName := s.nextAvailableModelName("new-model")
s.addModel(
picoclawconfig.ModelConfig{ModelName: newName, Model: "openai/gpt-5.2"},
)
s.push(
fmt.Sprintf("model-%d", len(s.config.ModelList)-1),
s.modelForm(len(s.config.ModelList)-1),
)
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The "Add model" action mutates s.config.ModelList via s.addModel(...) but does not set s.dirty = true. As a result, the user can add a model and then exit without being prompted to apply/save, losing the change. Mark the state dirty (and refresh menus if needed) when adding a model.

Copilot uses AI. Check for mistakes.
newName := s.nextAvailableModelName("new-model")
s.addModel(
picoclawconfig.ModelConfig{ModelName: newName, Model: "openai/gpt-5.2"},
)
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

Same issue as above in the menu refresh path: this "Add Model" action appends to ModelList but doesn’t mark s.dirty = true, so the new entry can be lost on exit without an apply prompt. Set s.dirty (and refresh relevant menus) when adding the new model here too.

Suggested change
)
)
s.dirty = true
refreshModelMenuFromState(menu, s)

Copilot uses AI. Check for mistakes.
SetDoneFunc(func(buttonIndex int, buttonLabel string) {
s.pages.RemovePage(pageName)
if buttonLabel == "Delete" {
s.deleteModel(index)
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

Deleting a model here ultimately only removes it from ModelList and pops the page; it doesn’t mark s.dirty = true. That means a delete can be silently discarded if the user exits without applying/saving. Also consider clearing/updating Agents.Defaults.Model if the deleted model was selected, and refreshing the main/model menus after deletion to avoid stale UI state.

Suggested change
s.deleteModel(index)
s.deleteModel(index)
// Mark configuration as dirty so the deletion is persisted.
s.dirty = true
// If the deleted model was the default, clear the default model reference.
if s.config != nil {
if s.config.Agents.Defaults.Model == model.Name {
s.config.Agents.Defaults.Model = ""
}
}

Copilot uses AI. Check for mistakes.
@imguoguo imguoguo merged commit ead2236 into sipeed:main Mar 9, 2026
8 checks passed
fishtrees pushed a commit to fishtrees/picoclaw that referenced this pull request Mar 12, 2026
…d#1271)

* fix(tui): fix model selection and enforce unique model_name, also fix model form button highlight

* feat(tui): add footer view with navigation instructions and update menu structure

* fix(tui): update model selection labels for clarity and consistency

* refactor(tui): remove unused rootChannelDescription function

* refactor(tui): simplify rootModelDescription and remove unused 'q' event handling in channel menu

* fix(tui): keep selected model name updated

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
dj-oyu pushed a commit to dj-oyu/picoclaw that referenced this pull request Mar 14, 2026
…d#1271)

* fix(tui): fix model selection and enforce unique model_name, also fix model form button highlight

* feat(tui): add footer view with navigation instructions and update menu structure

* fix(tui): update model selection labels for clarity and consistency

* refactor(tui): remove unused rootChannelDescription function

* refactor(tui): simplify rootModelDescription and remove unused 'q' event handling in channel menu

* fix(tui): keep selected model name updated

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
dj-oyu pushed a commit to dj-oyu/picoclaw that referenced this pull request Mar 16, 2026
…d#1271)

* fix(tui): fix model selection and enforce unique model_name, also fix model form button highlight

* feat(tui): add footer view with navigation instructions and update menu structure

* fix(tui): update model selection labels for clarity and consistency

* refactor(tui): remove unused rootChannelDescription function

* refactor(tui): simplify rootModelDescription and remove unused 'q' event handling in channel menu

* fix(tui): keep selected model name updated

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants