🤖 fix: align skill autocomplete matching (#3214)

## Summary

Align slash and inline skill autocomplete behavior so `` matches
`-simplifier` the same way `/simpl` does, and inline skill completions
leave the cursor ready for continued typing.

## Background

Slash skill suggestions already matched full-name prefixes and
hyphen-segment prefixes, but inline `` suggestions only matched
full-name prefixes. Inline completions also inserted no trailing space
at end-of-input, unlike slash completions.

## Implementation

- Added a shared `matchesHyphenatedPrefix` helper for full-prefix and
hyphen-segment-prefix matching.
- Reused that helper from slash and inline skill suggestion filtering
without changing ordering or introducing fuzzy matching.
- Updated inline skill insertion spacing so end-of-input completions
append one trailing space while preserving existing whitespace and
punctuation behavior.

## Validation

- `bun test src/browser/utils/slashCommands/suggestions.test.ts
src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts`
- `make static-check`
- Dogfooded in a dev-server sandbox with screenshots for `/simpl`, ``,
and punctuation-adjacent insertion.

## Risks

Low risk: the shared matcher preserves the previous slash predicate,
inline changes are covered by focused behavior tests, and
punctuation/whitespace insertion behavior remains explicitly tested.

---

<details>
<summary>📋 Implementation Plan</summary>

# Plan: align `/SKILL_NAME` and `$SKILL_NAME` autocomplete UX

## Goal

Make slash-command skill invocations and inline dollar skill invocations
feel consistent in the chat input:

1. Selecting an inline `$SKILL_NAME` suggestion should leave the cursor
ready to continue typing, matching the `/SKILL_NAME ` behavior.
2. `$SKILL_NAME` suggestions should match skill names the same way slash
skill commands do, including hyphen-segment prefix matches such as
`simpl` → `code-simplifier`.

## Evidence gathered

Read-only investigation found the user's claims are valid:

- `/` suggestions are built in
`src/browser/utils/slashCommands/suggestions.ts`.
  - Matching: full prefix OR hyphen-segment prefix.
- Replacement: `/${definition.key}${appendSpace ? " " : ""}`, with
`appendSpace` defaulting to `true`.
- `$` suggestions are built in
`src/browser/utils/agentSkills/inlineSkillSuggestions.ts`.
  - Matching: strict `descriptor.name.startsWith(lowered)` only.
  - Replacement: `$${descriptor.name}` with no baked-in space.
- Selection handlers live in `src/browser/features/ChatInput/index.tsx`.
- `/` handler uses `setInput(suggestion.replacement)`; the replacement
already includes the trailing space.
- `$` handler splices `suggestion.replacement +
getInlineSkillInsertionTrailingText(after) + after`.
- `getInlineSkillInsertionTrailingText(after)` currently returns `""` at
end-of-input, so selecting `$code-simplifier` at the end does not add a
space.

## Advisor review incorporated

The advisor agreed with the shared-helper direction, with refinements:

- Put the matcher in a neutral location/name so generic slash commands
do not import from `agentSkills`.
- Be explicit that this is **not fuzzy matching**: preserve current
slash behavior exactly as case-insensitive full-prefix OR
hyphen-segment-prefix matching, with no scoring or reordering.
- Update tests in the same phase as each implementation change so the
plan does not intentionally create a failing intermediate step.
- Keep `$` replacement as `$${name}` and only change the trailing-text
helper; baking the space into `$` replacements would break
punctuation/adjacent-text cases.
- Add a ChatInput-level selection/cursor test only if an existing
harness makes it cheap; otherwise rely on helper tests plus dogfooding.

## Recommended approach — shared matcher plus `$` trailing-space
alignment

**Net product LoC estimate:** +10 to +25 LoC.

### Phase 1 — Introduce neutral hyphen-prefix matching and use it in
both flows

1. Add a tiny neutral helper rather than putting shared logic under
either feature-specific directory:
   - Preferred location: `src/browser/utils/suggestionMatching.ts`
   - Preferred name: `matchesHyphenatedPrefix(name, partial)`
2. Implement the existing `/` algorithm exactly:
   - Normalize to lowercase.
   - Match an empty partial.
   - Match if the whole name starts with the partial.
   - Match if any `-`-separated segment starts with the partial.
- Do **not** add fuzzy scoring, substring matching, sorting, Fuse, or
any ordering changes.

   ```ts
export function matchesHyphenatedPrefix(name: string, partial: string):
boolean {
     const normalizedPartial = partial.trim().toLowerCase();
     const normalizedName = name.toLowerCase();

     return normalizedPartial === "" ||
       normalizedName.startsWith(normalizedPartial) ||
normalizedName.split("-").some((segment) =>
segment.startsWith(normalizedPartial));
   }
   ```

3. Use the helper in:
   - `src/browser/utils/slashCommands/suggestions.ts`
   - `src/browser/utils/agentSkills/inlineSkillSuggestions.ts`
4. Preserve existing suggestion ordering; filtering should not sort or
score results.
5. Be deliberate about `partial.trim()`:
   - This mirrors current slash behavior.
- Add or preserve coverage for empty/whitespace partials if inline
parser behavior makes whitespace partials possible.

**Phase 1 tests / quality gate:**

- Update/add tests in
`src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts`:
  - `$simpl` returns `$code-simplifier` via the `simplifier` segment.
- `$review` returns `$deep-review` or equivalent via the `review`
segment.
- Substring-only matches still do **not** match, e.g. `view` does not
match `deep-review`.
  - Result order follows descriptor order.
- Keep existing slash suggestion tests passing to confirm no slash
behavior changed.

```sh
bun test src/browser/utils/slashCommands/suggestions.test.ts src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts
```

### Phase 2 — Make `$` selection append a usable trailing space at
end-of-input

1. Update only `getInlineSkillInsertionTrailingText(after)` in
`src/browser/utils/agentSkills/inlineSkillSuggestions.ts`:

   ```ts
export function getInlineSkillInsertionTrailingText(after: string): "" |
" " {
     return after.length === 0
       ? " "
       : INLINE_SKILL_INSERT_EXISTING_SEPARATOR_RE.test(after[0] ?? "")
         ? ""
         : " ";
   }
   ```

2. Keep `getInlineSkillSuggestions()` replacements as
`$${descriptor.name}`. The `$` flow must continue deciding spacing at
insertion time because it can complete before punctuation or adjacent
text.
3. Preserve smart-skip behavior when the next character is already
whitespace, punctuation, or a closer, so `$skill,` does not become
`$skill ,`.
4. Confirm `src/browser/features/ChatInput/index.tsx` already computes
cursor position using `suggestion.replacement.length + trailing.length`;
only touch it if the current cursor math fails the new EOF-space
behavior.

**Phase 2 tests / quality gate:**

- Update trailing-text tests in
`src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts`:
  - `after === ""` → `" "`.
  - `after === " "` → `""`.
  - `after === ","` and `after === ")"` → `""`.
  - `after === "next"` → `" "`.
- If an existing ChatInput test harness supports this cheaply, add one
integration-ish selection test for `$simpl` that confirms the final
value/cursor lands after `$code-simplifier `.
- Do not build a new heavy harness just for this; helper tests plus
dogfooding are sufficient if no harness exists.

```sh
bun test src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts
```

### Phase 3 — Full local validation

Run validation after the product and test changes are complete:

```sh
make typecheck
make lint
bun test src/browser/utils/slashCommands/suggestions.test.ts src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts
```

If the branch already has broader UI-related changes, prefer `make
static-check` before handoff.

## Dogfooding plan

Because this is chat-input UX, dogfood in the actual app after tests
pass.

1. Start the app in a clean-ish dev environment:

   ```sh
   make dev
   ```

2. Use `agent-browser` to interact with the running app:
   - Open the Electron/web dev URL exposed by the dev command.
   - Navigate to a workspace chat input.
- Type `/simpl`, select `code-simplifier`, press Enter, and confirm the
input remains `/code-simplifier ` with the cursor after the space.
- Type `$simpl`, select `code-simplifier`, press Enter, and confirm the
input becomes `$code-simplifier ` with the cursor after the space.
- Type `$simpl, thanks`, place the cursor after `simpl`, select
`code-simplifier`, and confirm the input becomes `$code-simplifier,
thanks` with no extra space before the comma.
- Type `$simplnext`, place the cursor after `simpl`, select
`code-simplifier`, and confirm the input becomes `$code-simplifier next`
with exactly one separating space before `next`.
- Optional but useful: submit `$code-simplifier ` and confirm the
trailing space does not break skill invocation parsing.
3. Capture reviewer evidence:
   - Save screenshots before and after selecting `/simpl`.
   - Save screenshots before and after selecting `$simpl`.
- Record a short video showing `/simpl`, `$simpl`, and one
punctuation/adjacent-text case with cursor placement.
- If the environment blocks screenshots or video, report the exact
blocker and still provide the strongest available evidence.

## Acceptance criteria

- `$simpl` surfaces `$code-simplifier` when `code-simplifier` is an
available skill.
- `$` and `/` suggestions use the same case-insensitive full-prefix and
hyphen-segment-prefix matching behavior.
- Matching remains prefix/segment-prefix only; substring-only input such
as `view` does not match `deep-review`.
- Suggestion order is preserved; no fuzzy scoring or sorting is
introduced.
- Selecting `$code-simplifier` at the end of input inserts
`$code-simplifier `, including exactly one trailing space.
- Selecting `$code-simplifier` before whitespace or punctuation does not
insert an extra unwanted space.
- Existing slash command behavior remains unchanged.
- Targeted unit tests pass.
- Manual dogfooding demonstrates both matching and trailing-space
behavior with screenshots and a video recording unless the environment
prevents capture, in which case the blocker is documented.

## Risks and mitigations

- **Risk:** Changing `$` end-of-input behavior may surprise users who
intentionally want `$skill` with no trailing space.
- **Mitigation:** This aligns with existing `/` behavior and normal
autocomplete UX; punctuation/closer cases still avoid unwanted spaces.
- **Risk:** Sharing matching with slash commands may unintentionally
alter non-skill slash commands.
- **Mitigation:** Make the helper generic, preserve the exact current
slash predicate, preserve ordering, and keep slash tests passing.
- **Risk:** `partial.trim()` may subtly change inline `$` behavior if
whitespace partials are possible.
- **Mitigation:** This intentionally mirrors current slash behavior; add
coverage or verify parser behavior for empty/whitespace partials during
implementation.
- **Risk:** Over-refactoring suggestion code could expand the diff.
- **Mitigation:** Prefer one tiny neutral helper and surgical call-site
changes only.

## Alternative approaches considered

### Alternative A — Patch `$` only, leave `/` implementation unchanged

**Net product LoC estimate:** +5 to +15 LoC.

- Change `$` matching to copy the slash segment-aware predicate inline.
- Change `$` trailing helper to return a space at end-of-input.
- Pros: smallest possible diff and acceptable if a neutral helper
creates awkward imports or broad churn.
- Cons: duplicates matching logic and allows the two flows to drift
again.

### Alternative B — Preserve `$` smart no-space-at-EOL behavior, only
fix matching

**Net product LoC estimate:** +3 to +10 LoC.

- Only update `$` matching so `$simpl` works.
- Pros: very low risk.
- Cons: leaves the reported trailing-space inconsistency unresolved.

### Recommendation

Use the recommended shared-helper approach. It remains small, removes
the observed mismatch, preserves slash behavior, and prevents future
drift between slash skill invocation and inline skill invocation
matching. If the shared helper unexpectedly causes a large diff, fall
back to Alternative A rather than expanding the scope.

</details>

---

_Generated with `mux` • Model: `openai:gpt-5.5` • Thinking: `high` •
Cost: `1326901{MUX_COSTS_USD:-unknown}`_

<!-- mux-attribution: model=openai:gpt-5.5 thinking=high costs=11.97 -->

Author: ThomasK33

src/browser/utils/agentSkills/inlineSkillSuggestions.test.ts

@@ -46,6 +46,33 @@ describe("getInlineSkillSuggestions", () => {
     ).toEqual(["$tdd", "$tdd-review"]);
   });
 
+  test("matches hyphenated skill name segments", () => {
+    expect(
+      getInlineSkillSuggestions({
+        partial: "simpl",
+        descriptors: [descriptor("tdd"), descriptor("code-simplifier")],
+      }).map((suggestion) => suggestion.display)
+    ).toEqual(["$code-simplifier"]);
+  });
+
+  test("matches later hyphenated skill name segments", () => {
+    expect(
+      getInlineSkillSuggestions({
+        partial: "review",
+        descriptors: [descriptor("tdd"), descriptor("deep-review")],
+      }).map((suggestion) => suggestion.display)
+    ).toEqual(["$deep-review"]);
+  });
+
+  test("does not match substring-only partials", () => {
+    expect(
+      getInlineSkillSuggestions({
+        partial: "view",
+        descriptors: [descriptor("deep-review")],
+      }).map((suggestion) => suggestion.display)
+    ).toEqual([]);
+  });
+
   test("matches uppercase partials against canonical lowercase names", () => {
     expect(
       getInlineSkillSuggestions({
@@ -130,8 +157,11 @@ describe("getInlineSkillInsertionTrailingText", () => {
     }
   });
 
-  test("preserves existing whitespace and end-of-line behavior", () => {
-    expect(getInlineSkillInsertionTrailingText("")).toBe("");
+  test("adds a trailing space at the end of the input", () => {
+    expect(getInlineSkillInsertionTrailingText("")).toBe(" ");
+  });
+
+  test("preserves existing whitespace", () => {
     expect(getInlineSkillInsertionTrailingText(" next")).toBe("");
     expect(getInlineSkillInsertionTrailingText("\nnext")).toBe("");
   });

src/browser/utils/agentSkills/inlineSkillSuggestions.ts

@@ -1,3 +1,4 @@
+import { matchesNameBySegmentPrefix } from "@/browser/utils/suggestionMatching";
 import type { SlashSuggestion } from "@/browser/utils/slashCommands/types";
 import type { AgentSkillDescriptor } from "@/common/types/agentSkill";
 
@@ -29,18 +30,17 @@ export function shouldRefreshInlineSkillSuggestions(
 }
 
 export function getInlineSkillInsertionTrailingText(after: string): "" | " " {
-  // Characters where inserting a space before them would be wrong: whitespace,
-  // sentence punctuation, and closers that should bind to the skill reference.
-  return after.length === 0 || INLINE_SKILL_INSERT_EXISTING_SEPARATOR_RE.test(after[0] ?? "")
-    ? ""
-    : " ";
+  // At end-of-input, add a space so the cursor is ready for continued typing.
+  // Before whitespace, punctuation, or closers, skip the space to avoid doubling.
+  if (after.length === 0) return " ";
+  if (INLINE_SKILL_INSERT_EXISTING_SEPARATOR_RE.test(after[0] ?? "")) return "";
+  return " ";
 }
 
 /**
  * Returns suggestions for `$skill` autocomplete.
  *
- * - Filter rule: descriptor.name.startsWith(partial). Case-sensitive skill names are
- *   canonical lowercase IDs (validated by SkillNameSchema), so normalize the user's partial.
+ * - Filter rule: full-name prefix or hyphen-segment prefix, matching slash skill commands.
  * - Empty `partial` returns the full descriptor list (so typing just `$` opens the menu).
  * - Result order: descriptors order from caller (no re-sort). Caller already lists in
  *   scope-priority order.
@@ -50,9 +50,8 @@ export function getInlineSkillInsertionTrailingText(after: string): "" | " " {
 export function getInlineSkillSuggestions(
   context: InlineSkillSuggestionContext
 ): SlashSuggestion[] {
-  const lowered = context.partial.toLowerCase();
   return context.descriptors
-    .filter((descriptor) => descriptor.name.startsWith(lowered))
+    .filter((descriptor) => matchesNameBySegmentPrefix(descriptor.name, context.partial))
     .map((descriptor) => ({
       id: `inline-skill:${descriptor.name}`,
       display: `$${descriptor.name}`,

src/browser/utils/slashCommands/suggestions.ts

@@ -2,6 +2,7 @@
  * Slash command suggestions generation
  */
 
+import { matchesNameBySegmentPrefix } from "@/browser/utils/suggestionMatching";
 import { MODEL_ABBREVIATIONS } from "@/common/constants/knownModels";
 import { EXPERIMENT_IDS } from "@/common/constants/experiments";
 import { formatModelDisplayName } from "@/common/utils/ai/modelDisplay";
@@ -27,17 +28,10 @@ function filterAndMapSuggestions<T extends SuggestionDefinition>(
   build: (definition: T) => SlashSuggestion,
   filter?: (definition: T) => boolean
 ): SlashSuggestion[] {
-  const normalizedPartial = partial.trim().toLowerCase();
-
   return definitions
     .filter((definition) => {
       if (filter && !filter(definition)) return false;
-      const normalizedKey = definition.key.toLowerCase();
-      // Keep full-prefix matches (e.g., /deep-r) alongside segment matches (e.g., /r).
-      return normalizedPartial
-        ? normalizedKey.startsWith(normalizedPartial) ||
-            normalizedKey.split("-").some((segment) => segment.startsWith(normalizedPartial))
-        : true;
+      return matchesNameBySegmentPrefix(definition.key, partial);
     })
     .map((definition) => build(definition));
 }

src/browser/utils/suggestionMatching.test.ts

@@ -0,0 +1,22 @@
+import { describe, expect, test } from "bun:test";
+import { matchesNameBySegmentPrefix } from "./suggestionMatching";
+
+describe("matchesNameBySegmentPrefix", () => {
+  test("matches empty and whitespace-only partials", () => {
+    expect(matchesNameBySegmentPrefix("deep-review", "")).toBe(true);
+    expect(matchesNameBySegmentPrefix("deep-review", "   ")).toBe(true);
+  });
+
+  test("matches full-name prefixes case-insensitively", () => {
+    expect(matchesNameBySegmentPrefix("deep-review", "DEEP-R")).toBe(true);
+  });
+
+  test("matches hyphen-delimited segment prefixes", () => {
+    expect(matchesNameBySegmentPrefix("code-simplifier", "simpl")).toBe(true);
+    expect(matchesNameBySegmentPrefix("deep-review", "review")).toBe(true);
+  });
+
+  test("does not match substring-only partials", () => {
+    expect(matchesNameBySegmentPrefix("deep-review", "view")).toBe(false);
+  });
+});

src/browser/utils/suggestionMatching.ts

@@ -0,0 +1,16 @@
+/**
+ * Case-insensitive prefix match against a hyphenated name.
+ *
+ * Returns true when `partial` is a prefix of the full name or any
+ * hyphen-delimited segment. Empty or whitespace-only partials match everything.
+ */
+export function matchesNameBySegmentPrefix(name: string, partial: string): boolean {
+  const normalizedPartial = partial.trim().toLowerCase();
+  const normalizedName = name.toLowerCase();
+
+  return (
+    normalizedPartial.length === 0 ||
+    normalizedName.startsWith(normalizedPartial) ||
+    normalizedName.split("-").some((segment) => segment.startsWith(normalizedPartial))
+  );
+}