AI Assistant skill menu: one mixed chooser for cards + skill files

packages/host/app/components/ai-assistant/skill-menu/index.gts

@@ -1,6 +1,7 @@
 import { fn } from '@ember/helper';
 import { on } from '@ember/modifier';
 import { action } from '@ember/object';
+import { service } from '@ember/service';
 import Component from '@glimmer/component';
 
 import { tracked } from '@glimmer/tracking';
@@ -11,18 +12,19 @@ import pluralize from 'pluralize';
 
 import { Button } from '@cardstack/boxel-ui/components';
 
-import { baseRRI, skillCardRef } from '@cardstack/runtime-common';
-import { chooseCard, chooseFile } from '@cardstack/runtime-common';
+import { baseRRI, chooseCard, skillCardRef } from '@cardstack/runtime-common';
 
 import SkillToggle from '@cardstack/host/components/ai-assistant/skill-menu/skill-toggle';
 import PillMenu from '@cardstack/host/components/pill-menu';
 
-import type { RoomSkill } from '@cardstack/host/resources/room';
+import { skillsRealmURL } from '@cardstack/host/lib/utils';
 
-import type { FileDef } from '@cardstack/base/file-api';
+import type { RoomSkill } from '@cardstack/host/resources/room';
+import type RealmServerService from '@cardstack/host/services/realm-server';
 
 // A skill expressed as a markdown file is a `MarkdownDef` whose frontmatter
-// declares `boxel.kind: skill`. The file chooser is scoped to exactly those.
+// declares `boxel.kind: skill`. One branch of the mixed chooser's base filter
+// selects exactly those.
 const markdownDefRef = {
   module: baseRRI('markdown-file-def'),
   name: 'MarkdownDef',
@@ -75,8 +77,8 @@ export default class AiAssistantSkillMenu extends Component<Signature> {
           class='attach-button'
           @kind='primary'
           @size='extra-small'
-          {{on 'click' this.attachSkillCard}}
-          @disabled={{this.doAttachSkillCard.isRunning}}
+          {{on 'click' this.attachSkill}}
+          @disabled={{this.doAttachSkill.isRunning}}
           @loading={{this.isAttachingSkill}}
           data-test-pill-menu-add-button
         >
@@ -86,21 +88,6 @@ export default class AiAssistantSkillMenu extends Component<Signature> {
             Choose a Skill to add
           {{/if}}
         </Button>
-        <Button
-          class='attach-button'
-          @kind='primary'
-          @size='extra-small'
-          {{on 'click' this.attachSkillMarkdown}}
-          @disabled={{this.doAttachSkillMarkdown.isRunning}}
-          @loading={{this.isAttachingSkillMarkdown}}
-          data-test-pill-menu-add-markdown-button
-        >
-          {{#if this.isAttachingSkillMarkdown}}
-            Adding Skill
-          {{else}}
-            Choose a skill file to add
-          {{/if}}
-        </Button>
       </:footer>
     </PillMenu>
     <style scoped>
@@ -151,9 +138,10 @@ export default class AiAssistantSkillMenu extends Component<Signature> {
     </style>
   </template>
 
+  @service declare private realmServer: RealmServerService;
+
   @tracked private isExpanded = false;
   @tracked private isAttachingSkill = false;
-  @tracked private isAttachingSkillMarkdown = false;
 
   private urlForRealmLookup(skill: RoomSkill) {
     return skill.fileDef.sourceUrl;
@@ -184,53 +172,63 @@ export default class AiAssistantSkillMenu extends Component<Signature> {
   }
 
   @action
-  private attachSkillCard() {
-    this.doAttachSkillCard.perform();
+  private attachSkill() {
+    this.doAttachSkill.perform();
   }
 
-  private doAttachSkillCard = restartableTask(async () => {
-    let selectedCardIds =
+  // One chooser attaches either kind of skill: a Skill card or a skill-bearing
+  // markdown file (a MarkdownDef with `boxel.kind: skill`). The mixed chooser
+  // (`includeFiles`) surfaces both in a single list and tags each pick with its
+  // kind, which routes the result to the matching attach callback.
+  private doAttachSkill = restartableTask(async () => {
+    // Exclude already-attached skills, matched client-side against the menu's
+    // own skill list by `id`. Card skills are excluded immediately; file skills
+    // once their rows carry `id` in the search doc (Phase 1 reindex), and the
+    // feature ships without waiting on that reindex.
+    //
+    // The exclusion must be NULL-safe: a bare `not: { eq: { id } }` drops any
+    // row whose `id` is absent, because negated `eq` compiles to `NOT (id = X)`
+    // and `NOT (NULL = X)` is NULL (excluded) under three-valued logic. On a
+    // realm not yet carrying the stamped `id`, every skill-file row lacks `id`,
+    // so that would hide *all* skill files whenever any skill is attached. The
+    // `{ eq: { id: null } }` branch (an `IS NULL` test) keeps those rows.
+    let exclusions =
       this.args.skills?.map((skill: RoomSkill) => ({
-        not: { eq: { id: skill.cardId } },
+        any: [{ eq: { id: null } }, { not: { eq: { id: skill.cardId } } }],
       })) ?? [];
-    // query for only displaying skill cards that are not already selected
     let query = {
       filter: {
-        every: [{ type: skillCardRef }, ...selectedCardIds],
+        every: [
+          {
+            any: [
+              { type: skillCardRef },
+              { on: markdownDefRef, eq: { kind: 'skill' } },
+            ],
+          },
+          ...exclusions,
+        ],
       },
+      // Scope to the user's own workspaces plus the shared Boxel Skills realm,
+      // where most attachable skills live. A bounded scope keeps the mixed
+      // chooser (`includeFiles`) from fanning out across every server realm
+      // (base, catalog, …) and stalling on their contents.
+      realms: [
+        ...new Set([...this.realmServer.userRealmIdentifiers, skillsRealmURL]),
+      ],
     };
-    let cardId = await chooseCard(query);
-    if (cardId) {
-      try {
-        this.isAttachingSkill = true;
-        await this.args.onChooseCard?.(cardId);
-      } finally {
-        this.isAttachingSkill = false;
-      }
+    let chosen = await chooseCard(query, { includeFiles: true });
+    if (!chosen) {
+      return;
     }
-  });
-
-  @action
-  private attachSkillMarkdown() {
-    this.doAttachSkillMarkdown.perform();
-  }
-
-  // Parallel to doAttachSkillCard: pick a skill markdown file (a MarkdownDef
-  // with `boxel.kind: skill`) rather than a Skill card. The file chooser is
-  // scoped to skill markdown via the indexed `kind` field.
-  private doAttachSkillMarkdown = restartableTask(async () => {
-    let file = await chooseFile<FileDef>({
-      fileType: markdownDefRef,
-      fileTypeName: 'Skill',
-      fileFieldFilter: { kind: 'skill' },
-    });
-    if (file?.sourceUrl) {
-      try {
-        this.isAttachingSkillMarkdown = true;
-        await this.args.onChooseSkillMarkdown?.(file.sourceUrl);
-      } finally {
-        this.isAttachingSkillMarkdown = false;
+    try {
+      this.isAttachingSkill = true;
+      if (chosen.kind === 'file') {
+        await this.args.onChooseSkillMarkdown?.(chosen.id);
+      } else {
+        await this.args.onChooseCard?.(chosen.id);
       }
+    } finally {
+      this.isAttachingSkill = false;
     }
   });
 

packages/host/app/utils/card-search/query-builder.ts

@@ -60,12 +60,12 @@ function stripTypeFromFilter(filter: Filter): Filter | undefined {
     return on ? { every: children, on } : { every: children };
   }
   if (isAnyFilter(filter)) {
-    const children = filter.any
-      .map((f) => stripTypeFromFilter(f))
-      .filter((f): f is Filter => f !== undefined);
-    if (children.length === 0) return undefined;
-    if (children.length === 1) return on ? { ...children[0], on } : children[0];
-    return on ? { any: children, on } : { any: children };
+    // Never strip within an `any`: the branches are alternatives, so dropping
+    // a branch's type (or collapsing a stripped-empty branch) changes which
+    // rows match. e.g. the skill menu's `any: [{ type: skillCardRef }, { on:
+    // markdownDefRef, eq: { kind: 'skill' } }]` would collapse to the markdown
+    // branch alone, excluding skill cards. Keep the alternation intact.
+    return filter;
   }
   if (isNotFilter(filter)) {
     const inner = stripTypeFromFilter(filter.not);
@@ -211,8 +211,10 @@ export function buildSearchQuery(
   let filters: Filter[];
   if (baseFilter) {
     // When typeFilter is present, strip the baseFilter's (parent) type
-    // constraint as redundant — the picked subtype already implies it. Safe
-    // because the type picker only offers subtypes of the baseFilter type.
+    // constraint as redundant — the picked subtype already implies it.
+    // `stripTypeFromFilter` only unwraps a top-level `{ type }` node and leaves
+    // any `any`-alternation intact, so a compound base filter passes through
+    // whole.
     const effectiveBaseFilter = typeFilter
       ? stripTypeFromFilter(baseFilter)
       : baseFilter;

packages/host/tests/integration/components/ai-assistant-panel/skills-test.gts

@@ -10,6 +10,7 @@ import {
   REPLACE_MARKER,
   SEARCH_MARKER,
   SEPARATOR_MARKER,
+  ensureTrailingSlash,
   rri,
   skillCardRef,
 } from '@cardstack/runtime-common';
@@ -22,6 +23,7 @@ import {
 } from '@cardstack/runtime-common/matrix-constants';
 
 import OperatorMode from '@cardstack/host/components/operator-mode/container';
+import ENV from '@cardstack/host/config/environment';
 
 import type OperatorModeStateService from '@cardstack/host/services/operator-mode-state-service';
 
@@ -56,6 +58,8 @@ import { setupRenderingTest } from '../../../helpers/setup';
 
 import type { FileDef } from '@cardstack/base/file-api';
 
+const testRealmServerURL = ensureTrailingSlash(ENV.realmServerURL);
+
 module('Integration | ai-assistant-panel | skills', function (hooks) {
   const realmName = 'Operator Mode Workspace';
   let loader: Loader;
@@ -79,6 +83,11 @@ module('Integration | ai-assistant-panel | skills', function (hooks) {
   let mockMatrixUtils = setupMockMatrix(hooks, {
     loggedInAs: '@testuser:localhost',
     activeRealms: [testRealmURL],
+    // Populate the user's realm list so `realmServer.userRealmIdentifiers` is
+    // non-empty — the skill menu scopes its mixed chooser to those realms, and
+    // without this the search fans out across every server realm (the large
+    // shared skills realm included) and the intended tiles never render.
+    activeRealmServers: [testRealmServerURL],
     autostart: true,
     now: (() => {
       // deterministic clock so that, for example, screenshots
@@ -416,23 +425,19 @@ Instructions live in the markdown body.
     assert.dom('[data-test-skill-menu]').containsText('Skills: 2 of 2 active');
   });
 
-  test('skill picker can add a skill markdown file through the UI', async function (assert) {
+  test('skill picker can add a skill markdown file through the single mixed chooser', async function (assert) {
     const roomId = await renderAiAssistantPanel();
     let skillId = `${testRealmURL}realm-sync-skill.md`;
 
     await click('[data-test-skill-menu][data-test-pill-menu-button]');
-    await waitFor(
-      '[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]',
-    );
-    await click(
-      '[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]',
-    );
+    await waitFor('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+    await click('[data-test-skill-menu] [data-test-pill-menu-add-button]');
 
-    // The file chooser is scoped to skill markdown files.
-    await waitFor('[data-test-choose-file-modal]');
-    await waitFor('[data-test-file="realm-sync-skill.md"]');
-    await click('[data-test-file="realm-sync-skill.md"]');
-    await click('[data-test-choose-file-modal-add-button]');
+    // A skill markdown file surfaces as a result tile in the same mixed chooser
+    // as skill cards; picking it routes to the markdown-attach path by kind.
+    await waitFor(`[data-test-item-button="${skillId}"]`);
+    await click(`[data-test-item-button="${skillId}"]`);
+    await click('[data-test-card-chooser-go-button]');
 
     await waitUntil(
       () =>
@@ -468,37 +473,47 @@ Instructions live in the markdown body.
       );
   });
 
-  test('canceling the skill-file chooser re-enables the add-markdown button', async function (assert) {
+  test('the separate skill-file button is retired in favor of the single mixed chooser', async function (assert) {
     await renderAiAssistantPanel();
 
     await click('[data-test-skill-menu][data-test-pill-menu-button]');
-    await waitFor(
-      '[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]',
-    );
-    await click(
-      '[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]',
-    );
+    await waitFor('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+
+    assert
+      .dom('[data-test-skill-menu] [data-test-pill-menu-add-button]')
+      .exists('a single add button remains');
+    assert
+      .dom('[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]')
+      .doesNotExist(
+        'the second, file-tree skill button is gone — one chooser handles both kinds',
+      );
+  });
+
+  test('canceling the skill chooser re-enables the add button', async function (assert) {
+    await renderAiAssistantPanel();
+
+    await click('[data-test-skill-menu][data-test-pill-menu-button]');
+    await waitFor('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+    await click('[data-test-skill-menu] [data-test-pill-menu-add-button]');
 
-    await waitFor('[data-test-choose-file-modal]');
-    await click('[data-test-choose-file-modal-cancel-button]');
+    await waitFor('[data-test-card-chooser-modal]');
+    await click('[data-test-card-chooser-cancel-button]');
 
     // Canceling settles the chooser promise, so the trigger button returns to
     // its enabled state. A chooser that left its deferred unsettled would hang
     // the attach task and leave this button stuck disabled.
     await waitUntil(
-      () => !document.querySelector('[data-test-choose-file-modal]'),
+      () => !document.querySelector('[data-test-card-chooser-modal]'),
     );
     assert
-      .dom('[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]')
-      .isNotDisabled('the add-markdown button is re-enabled after canceling');
+      .dom('[data-test-skill-menu] [data-test-pill-menu-add-button]')
+      .isNotDisabled('the add button is re-enabled after canceling');
 
     // The chooser can be reopened after canceling.
-    await click(
-      '[data-test-skill-menu] [data-test-pill-menu-add-markdown-button]',
-    );
-    await waitFor('[data-test-choose-file-modal]');
+    await click('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+    await waitFor('[data-test-card-chooser-modal]');
     assert
-      .dom('[data-test-choose-file-modal]')
+      .dom('[data-test-card-chooser-modal]')
       .exists('the chooser reopens after canceling');
   });
 
@@ -529,6 +544,35 @@ Instructions live in the markdown body.
       .exists('a different skill remains available in the picker');
   });
 
+  test('skill picker excludes an already-attached markdown skill', async function (assert) {
+    await renderAiAssistantPanel();
+    let attachedMarkdownSkillId = `${testRealmURL}realm-sync-skill.md`;
+    let availableCardSkillId = `${testRealmURL}Skill/example`;
+
+    // The file row carries `id` in its search doc (Phase 1), so the same
+    // client-built `not: { eq: { id } }` exclusion that hides an attached card
+    // skill also hides an attached markdown skill from the mixed chooser.
+    await addSkillToAiAssistant(attachedMarkdownSkillId);
+
+    if (
+      !document.querySelector(
+        '[data-test-skill-menu] [data-test-pill-menu-add-button]',
+      )
+    ) {
+      await click('[data-test-skill-menu][data-test-pill-menu-button]');
+    }
+    await waitFor('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+    await click('[data-test-skill-menu] [data-test-pill-menu-add-button]');
+    await waitFor(`[data-test-item-button="${availableCardSkillId}"]`);
+
+    assert
+      .dom(`[data-test-item-button="${attachedMarkdownSkillId}"]`)
+      .doesNotExist('the already-attached markdown skill is excluded');
+    assert
+      .dom(`[data-test-item-button="${availableCardSkillId}"]`)
+      .exists('an unattached skill card remains available');
+  });
+
   test('skill pill menu opens the skill card', async function (assert) {
     await renderAiAssistantPanel();