Merge pull request #75931 from NousResearch/bb/tui-slash-priority

The TUI slash menu leads with the skills you actually use

tests/test_tui_gateway_server.py

@@ -6126,6 +6126,94 @@ def test_complete_slash_reasoning_includes_current_efforts_and_global_scope():
     assert {"max", "ultra", "--global"} <= values
 
 
+_SLASH_FILLER_COUNT = 60
+
+
+def _slash_skill_fixtures(monkeypatch):
+    """Stub a skill install big enough that a flat cap would truncate it."""
+    filler = {f"/filler-{i:03d}": 0 for i in range(_SLASH_FILLER_COUNT)}
+    usage = {"work": 297, "research": 84, "clean": 12}
+
+    monkeypatch.setattr(
+        server,
+        "_skill_usage_lookup",
+        lambda: (
+            lambda name: usage.get(name, 0),
+            lambda name: "bundled" if name.startswith("unused-") else "local",
+        ),
+    )
+    monkeypatch.setattr(
+        "agent.skill_commands.get_skill_commands",
+        lambda: {
+            "/work": {"description": "Fresh worktree"},
+            "/research": {"description": "Look it up"},
+            "/clean": {"description": "Polish the diff"},
+            "/unused-bundled": {"description": "Shipped, never opened"},
+            **{cmd: {"description": "Filler"} for cmd in filler},
+        },
+    )
+    monkeypatch.setattr("agent.skill_bundles.get_skill_bundles", lambda: {})
+
+
+def _slash_completions(text: str) -> list[dict]:
+    resp = server.handle_request(
+        {"id": "1", "method": "complete.slash", "params": {"text": text}}
+    )
+    return resp["result"]["items"]
+
+
+def test_complete_slash_offers_skills_alongside_commands(monkeypatch):
+    """A bare `/` must reach the skills, not just the registry.
+
+    The completer emits every registry command before the first skill, so one
+    flat cap spent every row on commands and no skill was reachable at all.
+    """
+    _slash_skill_fixtures(monkeypatch)
+
+    kinds = {item["kind"] for item in _slash_completions("/")}
+
+    assert kinds == {"command", "skill"}
+
+
+def test_complete_slash_ranks_skills_by_recorded_usage(monkeypatch):
+    """The skills someone actually invokes lead the ones they never opened."""
+    _slash_skill_fixtures(monkeypatch)
+
+    skills = [
+        item["text"].strip() for item in _slash_completions("/") if item["kind"] == "skill"
+    ]
+
+    assert skills[:3] == ["work", "research", "clean"]
+
+
+def test_complete_slash_prunes_unused_builtins_only_while_browsing(monkeypatch):
+    """A bare `/` is browsing and may prune; a typed query is a search.
+
+    A search that hides a match is broken, so the never-opened bundled skill
+    disappears from `/` and comes straight back the moment it is typed for.
+    """
+    _slash_skill_fixtures(monkeypatch)
+
+    browsing = {item["text"].strip() for item in _slash_completions("/")}
+    searching = {item["text"].strip() for item in _slash_completions("/unused")}
+
+    assert "unused-bundled" not in browsing
+    assert "unused-bundled" in searching
+
+
+def test_complete_slash_leaves_argument_stages_alone(monkeypatch):
+    """Ranking applies to the command token, never to a command's own args.
+
+    `/details c` completes that command's modes; a skill named /clean also
+    starts with a `c` and must not be offered as one of them.
+    """
+    _slash_skill_fixtures(monkeypatch)
+
+    items = _slash_completions("/details c")
+
+    assert [item["text"] for item in items] == ["collapsed", "cycle"]
+
+
 def test_config_set_reasoning_updates_live_session_and_agent(tmp_path, monkeypatch):
     monkeypatch.setattr(server, "_hermes_home", tmp_path)
     (tmp_path / "config.yaml").write_text("agent:\n  reasoning_effort: medium\n", encoding="utf-8")

tui_gateway/methods_complete.py

@@ -259,7 +259,20 @@ def _(rid, params: dict) -> dict:
                 ),
             }
             for c in completer.get_completions(doc, None)
-        ][:30]
+        ]
+
+        # Rank and bound the list (see _rank_slash_completions) while a
+        # `/token` is under the cursor — the one stage skills are offered at.
+        # An argument stage (`/personality `, `/details c`) keeps the order
+        # its own command chose.
+        if text.rsplit(" ", 1)[-1].startswith("/"):
+            usage, origin_of = _skill_usage_lookup()
+            items = _rank_slash_completions(
+                items, usage, origin_of, browsing=text == "/"
+            )
+        else:
+            items = items[:_SLASH_COMPLETION_LIMIT]
+
         text_lower = text.lower()
         extras = [
             {

tui_gateway/server.py

@@ -11402,6 +11402,55 @@ def _skill_usage_lookup():
     return usage, origin
 
 
+_SLASH_COMPLETION_LIMIT = 30
+
+
+def _rank_slash_completions(
+    items: list[dict],
+    usage,
+    origin_of,
+    *,
+    browsing: bool,
+) -> list[dict]:
+    """Rank and bound slash completions the way the menu should read.
+
+    ``usage``/``origin_of`` are the callables :func:`_skill_usage_lookup`
+    returns. Registry commands keep their existing order — only the skill
+    block is reordered, most-used first and A-Z within a tie, so the handful
+    of skills someone invokes daily lead the ones that shipped with Hermes
+    and were never opened.
+
+    The limit is spent PER KIND rather than on one flat truncation. A flat
+    cut is positional, not editorial: the completer emits every registry
+    command before the first skill, so on a 230-skill install a bare ``/``
+    hit the cap while still inside the command block and offered no skill at
+    all, and ``/p`` dropped ``/proving-a-fix-works`` (471 uses) while keeping
+    ``/pretext`` (2).
+
+    ``browsing`` separates the two things a slash means. A bare ``/`` is
+    BROWSING, so bundled skills with no recorded activity are dropped as
+    noise. A typed query is SEARCHING, and a search that hides a match is
+    broken — there nothing is pruned, the ranking only reorders.
+    """
+
+    def name_of(item: dict) -> str:
+        return str(item.get("text", "")).strip().lstrip("/").lower()
+
+    commands = [item for item in items if item.get("kind") != "skill"]
+    skills = [item for item in items if item.get("kind") == "skill"]
+
+    if browsing:
+        skills = [
+            item
+            for item in skills
+            if origin_of(name_of(item)) != "bundled" or usage(name_of(item)) > 0
+        ]
+
+    skills.sort(key=lambda item: (-usage(name_of(item)), name_of(item)))
+
+    return commands[:_SLASH_COMPLETION_LIMIT] + skills[:_SLASH_COMPLETION_LIMIT]
+
+
 def _cli_exec_blocked(argv: list[str]) -> str | None:
     """Return user hint if this argv must not run headless in the gateway process."""
     if not argv:

ui-tui/src/__tests__/inlineSlashSkill.test.ts

@@ -64,6 +64,28 @@ describe('completionRequestForInput — inline skill references', () => {
     })
   })
 
+  it('completes a second slash in a line that starts with a command', () => {
+    // Only the first slash is an invocation. Routing the whole line to the
+    // completer offered nothing, so `/work /cle` went dead while
+    // `do /work then /cle` completed fine.
+    expect(completionRequestForInput('/work /cle')).toMatchObject({
+      method: 'complete.slash',
+      params: { text: '/cle' },
+      replaceFrom: 7,
+      skillsOnly: true
+    })
+  })
+
+  it('leaves a command own arguments to the command', () => {
+    for (const input of ['/personality alic', '/cron ad', '/details ']) {
+      expect(completionRequestForInput(input)).toEqual({
+        method: 'complete.slash',
+        params: { text: input },
+        replaceFrom: 1
+      })
+    }
+  })
+
   it('routes a real mid-message path to path completion, not skills', () => {
     expect(completionRequestForInput('open src/foo/ba')).toMatchObject({ method: 'complete.path' })
     expect(completionRequestForInput('open /usr/lo')).toMatchObject({ method: 'complete.path' })

ui-tui/src/hooks/useCompletion.ts

@@ -41,15 +41,13 @@ export function completionRequestForInput(
     return null
   }
 
-  if (isSlashCommand) {
-    return { method: 'complete.slash', params: { text: input }, replaceFrom: 1 }
-  }
-
-  // A `/token` mid-message is a skill reference dropped into prose. It's only
-  // reachable here because the path branch below would otherwise claim it: a
-  // bare `/cle` matches TAB_PATH_RE as an absolute path. Skills win that tie —
-  // the moment a second `/` is typed the inline trigger stops matching and
-  // path completion takes back over.
+  // A `/token` mid-message is a skill reference dropped into prose. Detected
+  // BEFORE the leading-command shape because only the first slash can be an
+  // invocation — `/help /cle` is a command whose argument names a skill, and
+  // routing the whole line to the backend's completer offered nothing at all.
+  // It only matches a whitespace-preceded slash sitting at the caret, so
+  // ordinary argument completion (`/cron ad`, `/personality alic`) is
+  // untouched.
   const inline = inlineSlashTrigger(input)
 
   if (inline) {
@@ -61,6 +59,10 @@ export function completionRequestForInput(
     }
   }
 
+  if (isSlashCommand) {
+    return { method: 'complete.slash', params: { text: input }, replaceFrom: 1 }
+  }
+
   if (!pathWord) {
     return null
   }