Skip to content

Commit f6f2d83

Browse files
committed
feat: remove redundant List commands, add browsable pipeline stages to the palette
1. "Skill: List installed" removed - typing "skill" in the palette already shows every skill with its current state via SkillCommandProvider, making the separate one-shot dump-to- conversation command redundant. _refresh_installed_skills() no longer has a verbose parameter - always writes the terse "Skills found: N active M inactive" summary (used at startup); the cache- population role stays the same. 2. New PipelineCommandProvider: typing "pipeline" in the palette shows every stage from mycroft.conf's intents.pipeline array, numbered in order - "Pipeline: 1. stop_high", "Pipeline: 2. ovos-common- reading-pipeline-plugin", etc. Reads config fresh on every search (not cached, unlike installed_skills) since it's a fast local read, not a bus round-trip, and pipeline order essentially never changes without an OVOS restart. Selecting an entry writes a confirmation line to the conversation pane - read-only, no state to change. Replaces the old one-shot "Pipeline: List" command, same reasoning as removing "Skill: List installed". Looked into whether pipeline stages can be activated/deactivated at runtime like skills/services - found no confirmed live bus message for it (the official OVOS message spec has nothing pipeline-stage- specific, and pipeline order appears to be read once at startup, not re-read per-utterance). Toggling a stage would mean editing mycroft.conf itself, the same risky territory already flagged in issue #6 - filed as its own follow-up (#20) rather than guessed at or silently dropped. 185 tests passing (rewrote the removed-command tests, added Pipeline CommandProvider coverage); verified sdist->wheel build. Not tagging yet - commit+push freely, tag once we're both happy.
1 parent 6aace0c commit f6f2d83

4 files changed

Lines changed: 160 additions & 139 deletions

File tree

README.md

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -101,26 +101,29 @@ on modern Python/setuptools).
101101
one runs it immediately; the result ("ovos-core.service:
102102
restarted", or the failure reason) appears in the conversation
103103
pane.
104-
- **`Skill: `** - "Skill: List installed" (a static entry - fetches
105-
the current list via the bus and writes it to the conversation
106-
pane, one skill per line with its active/inactive/unknown state,
107-
refreshing the autocomplete source for the next two), plus "Skill:
108-
Activate <skill_id>" / "Skill: Deactivate <skill_id>" for every
109-
skill from that list, fuzzy-matched the same way as services -
110-
**only the relevant action is offered per skill** (Activate if
111-
inactive, Deactivate if active; both if the state is genuinely
112-
unknown), same as `Service:` above. Confirmed directly against a
113-
live OVOS instance that `skillmanager.list`'s response really does
114-
carry per-skill active state, not just names. Fire-and-forget (see
115-
`bus.py`'s honesty note on `activate_skill()`/`deactivate_skill()`
116-
- based on the documented mycroft-core convention) - the
117-
conversation-pane line confirms the request was *sent*, not that
118-
OVOS applied it; the local cache updates optimistically so the
119-
next search reflects the change immediately.
120-
- **`Pipeline: List`** - reads `mycroft.conf`'s `intents.pipeline`
121-
order via `ovos-config` (respects config layering) and writes it,
122-
numbered, to the conversation pane - a quick way to check pipeline
123-
order without leaving the TUI. Read-only.
104+
- **`Skill: `** - one entry per known skill, current state shown in
105+
the title - "Skill: skill_id (Active)" or "Skill: skill_id
106+
(Inactive)" - selecting it toggles (Deactivate if currently
107+
Active, Activate if currently Inactive), same "state in title,
108+
selecting toggles" convention as `Log:` above. An unknown state
109+
shows both Activate and Deactivate explicitly instead, since
110+
there's no current state to toggle from. The list is populated
111+
once at startup via the bus (`skillmanager.list` - confirmed
112+
directly against a live OVOS instance that its response really
113+
does carry per-skill active state, not just names), not refetched
114+
per keystroke. Fire-and-forget (see `bus.py`'s honesty note on
115+
`activate_skill()`/`deactivate_skill()` - based on the documented
116+
mycroft-core convention) - the conversation-pane line confirms the
117+
request was *sent*, not that OVOS applied it; the local cache
118+
updates optimistically so the next search reflects the change
119+
immediately. No separate "list installed skills" command anymore -
120+
typing "skill" already shows every one of them.
121+
- **`Pipeline: `** - one entry per stage in `mycroft.conf`'s
122+
`intents.pipeline` array, numbered in order - "Pipeline: 1.
123+
stop_high", "Pipeline: 2. ovos-common-reading-pipeline-plugin",
124+
etc, via `ovos-config` (respects config layering). Read-only -
125+
pipeline order is static config, not something with a live bus
126+
toggle; selecting an entry just confirms which stage you found.
124127
- Textual's own default **"Screenshot"** and **"Keys"** commands are
125128
filtered out - Screenshot isn't useful for this tool; Keys is
126129
Textual's own default trigger for exactly the same show-help-panel

ovos_tui_client/app.py

Lines changed: 76 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,52 @@ async def search(self, query: str) -> Hits:
174174
)
175175

176176

177+
class PipelineCommandProvider(Provider):
178+
"""Command Palette provider (Ctrl+P) for browsing the intent
179+
pipeline order (mycroft.conf's intents.pipeline array) - typing
180+
"pipeline" shows every stage with its position, e.g. "Pipeline:
181+
1. stop_high", "Pipeline: 2. ovos-common-reading-pipeline-plugin".
182+
Replaces the old one-shot "Pipeline: List" command that dumped the
183+
whole list to the conversation pane in one go - redundant once the
184+
list is directly browsable/searchable here, same reasoning as
185+
removing "Skill: List installed" once "Skill: " search already
186+
shows every skill.
187+
188+
Read directly from config on every search call rather than cached -
189+
unlike installed_skills (a real bus round-trip with a timeout),
190+
reading mycroft.conf is fast/local, and pipeline order essentially
191+
never changes without an OVOS restart, so there's no staleness
192+
concern worth caching against. Uses ovos_config (respects OVOS's
193+
own config layering) rather than parsing the raw file.
194+
195+
Read-only - toggling stages on/off would mean writing to
196+
mycroft.conf itself, which needs its own careful design pass (see
197+
issue #6 on viewing/editing mycroft.conf generally, and the
198+
follow-up issue on pipeline-stage toggling specifically - no
199+
confirmed live bus message exists for this, only static config).
200+
Selecting a hit here just confirms which stage you found by
201+
writing it to the conversation pane - there's no state to change."""
202+
203+
async def search(self, query: str) -> Hits:
204+
matcher = self.matcher(query)
205+
try:
206+
from ovos_config.config import Configuration
207+
pipeline = Configuration().get("intents", {}).get("pipeline", [])
208+
except Exception:
209+
return
210+
for i, stage in enumerate(pipeline, start=1):
211+
command_text = f"Pipeline: {i}. {stage}"
212+
score = matcher.match(command_text)
213+
if score > 0:
214+
yield Hit(
215+
score,
216+
matcher.highlight(command_text),
217+
partial(self._show, i, stage),
218+
)
219+
220+
def _show(self, position: int, stage: str) -> None:
221+
self.app._write_status(f"Pipeline stage {position}: {stage}")
222+
177223

178224
class ClickableLabel(Label):
179225
"""A Label that also responds to being clicked, so it can double
@@ -249,13 +295,12 @@ class SkillCommandProvider(Provider):
249295
"""Command Palette provider (Ctrl+P) for activating/deactivating
250296
any known installed skill, same in-place filtering as
251297
ServiceCommandProvider above - no modal window. Hits read from
252-
OVOSTUIApp.installed_skills, a cache populated by
253-
_refresh_installed_skills() (called once at startup, and again
254-
whenever "Skill: List installed" runs - see get_system_commands())
255-
rather than fetched fresh on every keystroke, since that would
256-
mean a bus round-trip per character typed. If the cache is still
257-
empty (nothing successfully fetched yet), this simply has no hits
258-
to offer yet - not an error, just nothing to show.
298+
OVOSTUIApp.installed_skills, a cache populated once at startup by
299+
_refresh_installed_skills() (see on_mount()) rather than fetched
300+
fresh on every keystroke, since that would mean a bus round-trip
301+
per character typed. If the cache is still empty (nothing
302+
successfully fetched yet), this simply has no hits to offer yet -
303+
not an error, just nothing to show.
259304
260305
installed_skills maps skill_id -> active (True/False/None -
261306
confirmed directly against a live OVOS instance: 'skillmanager.
@@ -312,7 +357,7 @@ def _run(self, method_name: str, skill_id: str, label: str) -> None:
312357
# optimistic local update - fire-and-forget means there's no
313358
# confirmation to wait for anyway, and this means the very
314359
# next search only offers the OTHER action for this skill,
315-
# without needing a full re-fetch via "Skill: List installed"
360+
# without needing a full re-fetch from the bus
316361
self.app.installed_skills[skill_id] = (method_name == "activate_skill")
317362
self.app._write_status(f"{skill_id}: {label.lower()} requested")
318363

@@ -386,7 +431,7 @@ class OVOSTUIApp(App):
386431
# renumbering F5+ to fill the gap - no benefit to disrupting keys
387432
# that already work.
388433

389-
COMMANDS = App.COMMANDS | {ServiceCommandProvider, SkillCommandProvider, SkillFilterCommandProvider}
434+
COMMANDS = App.COMMANDS | {ServiceCommandProvider, SkillCommandProvider, SkillFilterCommandProvider, PipelineCommandProvider}
390435

391436
def __init__(self, host="127.0.0.1", port=8181, lang="en-us", log_dir_override=None):
392437
super().__init__()
@@ -500,7 +545,7 @@ def on_mount(self) -> None:
500545

501546
self._startup_steps_remaining = 2
502547
self._check_services_worker()
503-
self._refresh_installed_skills(verbose=False, on_complete=self._finish_startup)
548+
self._refresh_installed_skills(on_complete=self._finish_startup)
504549

505550
def _finish_startup(self) -> None:
506551
"""Called once each of the two async/off-thread startup steps
@@ -600,23 +645,22 @@ def _write_status(self, text: str, ok: bool = True) -> None:
600645
style = "dim" if ok else "red"
601646
self._write_conversation(f"[{style}]{text}[/{style}]")
602647

603-
def _refresh_installed_skills(self, verbose: bool = True, on_complete=None) -> None:
648+
def _refresh_installed_skills(self, on_complete=None) -> None:
604649
"""Populates self.installed_skills (SkillCommandProvider's
605650
autocomplete source) via bus.list_skills(). Called once at
606-
startup (verbose=False - active/inactive counts only, not a
607-
full listing - see on_mount's boot-narration docstring) and
608-
again whenever 'Skill: List installed' runs (see
609-
get_system_commands(), verbose=True by default) - not on every
610-
palette keystroke, since each call is a real bus round-trip
611-
with a timeout.
612-
613-
verbose=True writes one skill per line, with its active/
614-
inactive/unknown state, to the conversation pane (not a single
615-
comma-joined line) - readability, especially as the list
616-
grows. See issue #15 for the not-yet-implemented idea of
617-
grouping/categorizing by skill type (skill/ocp/reading/
618-
pipeline etc) - deferred pending research into whether that's
619-
reliably detectable from skill_id alone.
651+
startup, not on every palette keystroke, since each call is a
652+
real bus round-trip with a timeout - the cache is what
653+
SkillCommandProvider actually searches against.
654+
655+
Writes a terse "Skills found: N active M inactive" summary to
656+
the conversation pane - no separate full listing anymore
657+
(there used to be a "Skill: List installed" palette command
658+
for that; removed as redundant once typing "skill" in the
659+
palette already shows every skill with its current state, per
660+
SkillCommandProvider). See issue #15 for the not-yet-
661+
implemented idea of grouping/categorizing by skill type
662+
(skill/ocp/reading/pipeline etc) - deferred pending research
663+
into whether that's reliably detectable from skill_id alone.
620664
621665
on_complete, if given, is called (via call_from_thread, same
622666
as everything else here) after the result is written -
@@ -631,47 +675,17 @@ def _on_result(skills):
631675
self.call_from_thread(self._write_status, "Skill list: no response (timed out)", ok=False)
632676
else:
633677
self.installed_skills = skills
634-
if verbose:
635-
self.call_from_thread(self._write_status, f"Skills: {len(self.installed_skills)} installed:")
636-
for skill_id in sorted(self.installed_skills):
637-
active = self.installed_skills[skill_id]
638-
state = "active" if active else "inactive" if active is False else "unknown"
639-
self.call_from_thread(self._write_status, f" {skill_id} ({state})")
640-
else:
641-
n_active = sum(1 for v in self.installed_skills.values() if v)
642-
n_inactive = sum(1 for v in self.installed_skills.values() if v is False)
643-
n_unknown = sum(1 for v in self.installed_skills.values() if v is None)
644-
summary = f"Skills found: {n_active} active {n_inactive} inactive"
645-
if n_unknown:
646-
summary += f" {n_unknown} unknown"
647-
self.call_from_thread(self._write_status, summary)
678+
n_active = sum(1 for v in self.installed_skills.values() if v)
679+
n_inactive = sum(1 for v in self.installed_skills.values() if v is False)
680+
n_unknown = sum(1 for v in self.installed_skills.values() if v is None)
681+
summary = f"Skills found: {n_active} active {n_inactive} inactive"
682+
if n_unknown:
683+
summary += f" {n_unknown} unknown"
684+
self.call_from_thread(self._write_status, summary)
648685
if on_complete is not None:
649686
self.call_from_thread(on_complete)
650687
self.bus.list_skills(_on_result)
651688

652-
def _list_pipeline(self) -> None:
653-
"""'Pipeline: List' - reads mycroft.conf's intents.pipeline
654-
array via ovos_config (respects OVOS's own config layering,
655-
rather than parsing the raw file directly) and writes it to
656-
the conversation pane, one stage per line in order - a quick
657-
way to check pipeline order without leaving the TUI, which
658-
this project's own README has an entire section on getting
659-
right (see the earlier pause/stop-vocabulary debugging
660-
history). Read-only - see issue #6 for the separate, larger
661-
question of viewing/editing mycroft.conf more generally."""
662-
try:
663-
from ovos_config.config import Configuration
664-
pipeline = Configuration().get("intents", {}).get("pipeline", [])
665-
except Exception as e:
666-
self._write_status(f"Pipeline: could not read config - {e}", ok=False)
667-
return
668-
if not pipeline:
669-
self._write_status("Pipeline: intents.pipeline is empty or not set")
670-
return
671-
self._write_status(f"Pipeline ({len(pipeline)} stages, in order):")
672-
for i, stage in enumerate(pipeline, start=1):
673-
self._write_status(f" {i}. {stage}")
674-
675689

676690

677691
def _update_skills_status(self) -> None:
@@ -853,8 +867,6 @@ def get_system_commands(self, screen: Screen):
853867
if cmd.title not in ("Screenshot", "Keys"):
854868
yield cmd
855869
yield SystemCommand("Help: Toggle panel", "", self.action_toggle_help_panel)
856-
yield SystemCommand("Skill: List installed", "", self._refresh_installed_skills)
857-
yield SystemCommand("Pipeline: List", "", self._list_pipeline)
858870
yield SystemCommand("Focus: Logs", "", self.action_focus_logs)
859871
yield SystemCommand("Focus: Conversation", "", self.action_focus_conversation)
860872
yield SystemCommand("Focus: Activity", "", self.action_focus_activity)

0 commit comments

Comments
 (0)