Skip to content

Enhance model selection and add footer navigation instructions #1271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
imguoguo merged 6 commits into from
Mar 9, 2026
Merged

Enhance model selection and add footer navigation instructions #1271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b23ecc0
fix(tui): fix model selection and enforce unique model_name, also fix…
taorye Mar 9, 2026
05b1442
feat(tui): add footer view with navigation instructions and update me…
taorye Mar 9, 2026
7d39488
fix(tui): update model selection labels for clarity and consistency
taorye Mar 9, 2026
76f64a3
refactor(tui): remove unused rootChannelDescription function
taorye Mar 9, 2026
3a90e19
refactor(tui): simplify rootModelDescription and remove unused 'q' ev…
taorye Mar 9, 2026
6214f23
fix(tui): keep selected model name updated
taorye Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 36 additions & 20 deletions cmd/picoclaw-launcher-tui/internal/ui/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ui

import (
"fmt"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -67,6 +68,7 @@ func Run() error {
root := tview.NewFlex().SetDirection(tview.FlexRow)
root.AddItem(bannerView(), 6, 0, false)
root.AddItem(state.pages, 0, 1, true)
root.AddItem(footerView(), 1, 0, false)

if err := state.app.SetRoot(root, true).EnableMouse(false).Run(); err != nil {
return err
Expand Down Expand Up @@ -102,18 +104,15 @@ func (s *appState) pop() {
}

func (s *appState) mainMenu() tview.Primitive {
menu := NewMenu("Config Menu", nil)
menu := NewMenu("Menu", nil)
refreshMainMenu(menu, s)
menu.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey {
switch event.Key() {
case tcell.KeyEsc:
s.requestExit()
return nil
}
if event.Rune() == 'q' {
s.requestExit()
return nil
}

return event
})

Expand All @@ -131,6 +130,32 @@ func (s *appState) refreshMenu(name string, menu *Menu) {
}
}

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,
}
Comment on lines +133 to +149
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.
total = len(entries)
for _, v := range entries {
if v {
enabled++
}
}
return enabled, total
}

func refreshMainMenuIfPresent(s *appState) {
if menu, ok := s.menus["main"]; ok {
refreshMainMenu(menu, s)
Expand All @@ -141,6 +166,7 @@ func refreshMainMenu(menu *Menu, s *appState) {
selectedModel := s.selectedModelName()
modelReady := selectedModel != ""
channelReady := s.hasEnabledChannel()
enabledCount, totalChannels := s.countChannels()
gatewayRunning := s.gatewayCmd != nil || s.isGatewayRunning()

gatewayLabel := "Start Gateway"
Expand All @@ -153,7 +179,7 @@ func refreshMainMenu(menu *Menu, s *appState) {
items := []MenuItem{
{
Label: rootModelLabel(selectedModel),
Description: rootModelDescription(selectedModel),
Description: rootModelDescription(),
Action: func() {
s.push("model", s.modelMenu())
},
Expand All @@ -167,7 +193,7 @@ func refreshMainMenu(menu *Menu, s *appState) {
},
{
Label: rootChannelLabel(channelReady),
Description: rootChannelDescription(channelReady),
Description: fmt.Sprintf("%d/%d enabled", enabledCount, totalChannels),
Action: func() {
s.push("channel", s.channelMenu())
},
Expand Down Expand Up @@ -311,16 +337,13 @@ func (s *appState) selectedModelName() string {

func rootModelLabel(selected string) string {
if selected == "" {
return "Model (no model selected)"
return "Model (None)"
}
return "Model (" + selected + ")"
}

func rootModelDescription(selected string) string {
if selected == "" {
return "no model selected"
}
return "selected"
func rootModelDescription() string {
return "Using SPACE to choose your model"
}

func rootChannelLabel(valid bool) string {
Expand All @@ -330,13 +353,6 @@ func rootChannelLabel(valid bool) string {
return "Channel"
}

func rootChannelDescription(valid bool) string {
if !valid {
return "no channel enabled"
}
return "enabled"
}

func (s *appState) startTalk() {
if !s.isActiveModelValid() {
s.showMessage("Model required", "Select a valid model before starting talk")
Expand Down
5 changes: 0 additions & 5 deletions cmd/picoclaw-launcher-tui/internal/ui/channel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

func (s *appState) buildChannelMenuItems() []MenuItem {
return []MenuItem{
{Label: "Back", Description: "Return to main menu", Action: func() { s.pop() }},
channelItem(
"Telegram",
"Telegram bot settings",
Expand Down Expand Up @@ -101,10 +100,6 @@ func (s *appState) channelMenu() tview.Primitive {
s.pop()
return nil
}
if event.Rune() == 'q' {
s.pop()
return nil
}
return event
})
return menu
Expand Down
140 changes: 96 additions & 44 deletions cmd/picoclaw-launcher-tui/internal/ui/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,7 @@ import (
)

func (s *appState) modelMenu() tview.Primitive {
items := make([]MenuItem, 0, 2+len(s.config.ModelList))
items = append(items,
MenuItem{Label: "Back", Description: "Return to main menu", Action: func() { s.pop() }},
MenuItem{
Label: "Add model",
Description: "Append a new model entry",
Action: func() {
s.addModel(
picoclawconfig.ModelConfig{ModelName: "new-model", Model: "openai/gpt-5.2"},
)
s.push(
fmt.Sprintf("model-%d", len(s.config.ModelList)-1),
s.modelForm(len(s.config.ModelList)-1),
)
},
},
)
items := make([]MenuItem, 0, 1+len(s.config.ModelList))
currentModel := strings.TrimSpace(s.config.Agents.Defaults.Model)
for i := range s.config.ModelList {
index := i
Expand All @@ -57,21 +41,35 @@ func (s *appState) modelMenu() tview.Primitive {
},
})
}
// 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.
items = append(items,
MenuItem{
Label: "**Add model**",
Description: "Append a new model entry",
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),
)
Comment on lines +49 to +57
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.
},
},
)

menu := NewMenu("Models", items)
menu.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey {
if event.Key() == tcell.KeyEsc {
s.pop()
return nil
}
if event.Rune() == 'q' {
s.pop()
return nil
}

if event.Rune() == ' ' {
row, _ := menu.GetSelection()
if row > 0 && row <= len(s.config.ModelList) {
model := s.config.ModelList[row-1]
if row >= 0 && row < len(s.config.ModelList) {
model := s.config.ModelList[row]
if !isModelValid(model) {
s.showMessage(
"Invalid model",
Expand All @@ -95,12 +93,23 @@ func (s *appState) modelForm(index int) tview.Primitive {
model := &s.config.ModelList[index]
form := tview.NewForm()
form.SetBorder(true).SetTitle(fmt.Sprintf("Model: %s", model.ModelName))
form.SetButtonBackgroundColor(tcell.NewRGBColor(80, 250, 123))
form.SetButtonTextColor(tcell.NewRGBColor(12, 13, 22))

addInput(form, "Model Name", model.ModelName, func(value string) {
if value == "" {
s.showMessage("Invalid model name", "Model Name cannot be empty")
return
}
if s.modelNameExists(value, index) {
s.showMessage("Duplicate model name", fmt.Sprintf("Model Name '%s' already exists", value))
return
}
Comment on lines +102 to +105
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.
oldName := model.ModelName
model.ModelName = value
if s.config.Agents.Defaults.Model == oldName {
s.config.Agents.Defaults.Model = value
}
s.dirty = true
form.SetTitle(fmt.Sprintf("Model: %s", model.ModelName))
refreshMainMenuIfPresent(s)
if menu, ok := s.menus["model"]; ok {
refreshModelMenuFromState(menu, s)
Expand Down Expand Up @@ -158,7 +167,21 @@ func (s *appState) modelForm(index int) tview.Primitive {
})

form.AddButton("Delete", func() {
s.deleteModel(index)
pageName := "confirm-delete-model"
if s.pages.HasPage(pageName) {
return
}
modal := tview.NewModal().
SetText("Are you sure you want to delete this model?").
AddButtons([]string{"Cancel", "Delete"}).
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.
}
})
modal.SetTitle("Confirm Delete").SetBorder(true)
s.pages.AddPage(pageName, modal, true, true)
})
form.AddButton("Test", func() {
s.testModel(model)
Expand Down Expand Up @@ -215,7 +238,7 @@ func modelStatusColor(valid bool, selected bool) *tcell.Color {

func refreshModelMenu(menu *Menu, currentModel string, models []picoclawconfig.ModelConfig) {
for i, model := range models {
row := i + 1
row := i
label := fmt.Sprintf("%s (%s)", model.ModelName, model.Model)
isValid := isModelValid(model)
if model.ModelName == currentModel && currentModel != "" {
Expand All @@ -234,23 +257,7 @@ func refreshModelMenu(menu *Menu, currentModel string, models []picoclawconfig.M
}

func refreshModelMenuFromState(menu *Menu, s *appState) {
items := make([]MenuItem, 0, 2+len(s.config.ModelList))
items = append(items,
MenuItem{Label: "Back", Description: "Return to main menu", Action: func() { s.pop() }},
MenuItem{
Label: "Add model",
Description: "Append a new model entry",
Action: func() {
s.addModel(
picoclawconfig.ModelConfig{ModelName: "new-model", Model: "openai/gpt-5.2"},
)
s.push(
fmt.Sprintf("model-%d", len(s.config.ModelList)-1),
s.modelForm(len(s.config.ModelList)-1),
)
},
},
)
items := make([]MenuItem, 0, 1+len(s.config.ModelList))
currentModel := strings.TrimSpace(s.config.Agents.Defaults.Model)
for i := range s.config.ModelList {
index := i
Expand All @@ -277,6 +284,19 @@ func refreshModelMenuFromState(menu *Menu, s *appState) {
},
})
}
items = append(items,
MenuItem{
Label: "**Add Model**",
Description: "Append a new model entry",
Action: func() {
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.
s.push(fmt.Sprintf("model-%d", len(s.config.ModelList)-1), s.modelForm(len(s.config.ModelList)-1))
},
},
)
menu.applyItems(items)
}

Expand All @@ -287,6 +307,38 @@ func isModelValid(model picoclawconfig.ModelConfig) bool {
return hasKey && hasModel
}

func (s *appState) modelNameExists(name string, excludeIndex int) bool {
target := strings.TrimSpace(name)
if target == "" {
return false
}
for i := range s.config.ModelList {
if i == excludeIndex {
continue
}
if strings.TrimSpace(s.config.ModelList[i].ModelName) == target {
return true
}
}
return false
}

func (s *appState) nextAvailableModelName(base string) string {
name := strings.TrimSpace(base)
if name == "" {
name = "new-model"
}
if !s.modelNameExists(name, -1) {
return name
}
for i := 2; ; i++ {
candidate := fmt.Sprintf("%s-%d", name, i)
if !s.modelNameExists(candidate, -1) {
return candidate
}
}
}

func (s *appState) testModel(model *picoclawconfig.ModelConfig) {
if model == nil {
return
Expand Down
12 changes: 12 additions & 0 deletions cmd/picoclaw-launcher-tui/internal/ui/style.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,15 @@ func bannerView() *tview.TextView {
text.SetBorder(false)
return text
}

const footerText = "Esc: Back/Exit | Enter: Enter | ←↓↑→ : Move | Space: Select | Tab/Shift+Tab: Switch"

func footerView() *tview.TextView {
text := tview.NewTextView()
text.SetTextAlign(tview.AlignCenter)
text.SetText(footerText)
text.SetBackgroundColor(tview.Styles.MoreContrastBackgroundColor)
text.SetTextColor(tview.Styles.PrimaryTextColor)
text.SetBorder(false)
return text
}
Loading