UX#1: FeatureInjectionBar — dedupe applied list into chips with ×N count

apstenku123 · apstenku123 · commit 6fe551830cd1 · 2026-05-23T21:47:44.000+02:00
Screenshot evidence: the bar at the top of the canvas showed
"mtp_weighted, mtp_weighted, mtp_weighted, mtp_weighted" as a
comma-joined string whenever the user clicked Apply for the same
injection more than once.

Fix:
- Group applied[] by name → render ONE chip per name with a "×N"
  count badge when N&gt;1.
- Each chip gets its own × remove button.
- New optional onRemove(injection) prop; App.tsx wires it to pop the
  last matching rewriter from spec.rewriters via dispatch.

Tests (3 new in FeatureInjectionBar.test.tsx, 7 total):
- 4 Apply clicks → 1 chip + count "×4", no comma-joined leak
- × remove fires onRemove with the popped injection, count drops
- single-instance chip has no count badge
diff --git a/vbgui/src/App.tsx b/vbgui/src/App.tsx
@@ -779,6 +779,31 @@ export function App(): JSX.Element {
     }
   }, [nodes, featureInjections.length]);
 
+  const handleFeatureInjectionRemove = useCallback((
+    injection: AppliedInjection,
+  ) => {
+    setFeatureInjections((prev) => {
+      const next = [...prev];
+      for (let i = next.length - 1; i >= 0; i--) {
+        if (next[i].name === injection.name) {
+          next.splice(i, 1);
+          break;
+        }
+      }
+      return next;
+    });
+    if (injection.paper_ref.startsWith("rewriter:")) {
+      const name = injection.paper_ref.slice("rewriter:".length);
+      let lastIdx = -1;
+      for (let i = spec.rewriters.length - 1; i >= 0; i--) {
+        if (spec.rewriters[i].name === name) { lastIdx = i; break; }
+      }
+      if (lastIdx >= 0) {
+        dispatch({ type: "rewriters.remove", index: lastIdx });
+      }
+    }
+  }, [spec.rewriters]);
+
   // V8-R02: apply a scaled-down preset from the GalleryScaleDownSlider.
   // The slider already ran architectures.scale_down and hands us back
   // the wire-form specs + chosen (hidden_size, num_layers). We swap the
@@ -1614,6 +1639,7 @@ export function App(): JSX.Element {
                   rpc={rpc}
                   applied={featureInjections}
                   onApply={handleFeatureInjectionApply}
+                  onRemove={handleFeatureInjectionRemove}
                 />
                 <DimEnvEditor value={dimEnv} onApply={setDimEnv} />
                 <TrainOptionsPanel
diff --git a/vbgui/src/components/FeatureInjectionBar.tsx b/vbgui/src/components/FeatureInjectionBar.tsx
@@ -14,6 +14,7 @@
 
 import { useEffect, useState } from "react";
 import type { RpcClient } from "@/lib/rpc";
+import { T } from "@/theme";
 
 interface CatalogOption {
   name: string;
@@ -32,10 +33,14 @@ export interface FeatureInjectionBarProps {
   /** Optional list of already-applied injections; the bar uses this
    *  to populate the applied-list display when the parent persists. */
   applied?: AppliedInjection[];
+  /** Optional remove callback. Called with the last-applied injection
+   *  matching the chip the user clicked × on. Parent is responsible
+   *  for popping the matching rewriter / brick. */
+  onRemove?: (injection: AppliedInjection) => void;
 }
 
 export function FeatureInjectionBar({
-  rpc, onApply, applied = [],
+  rpc, onApply, applied = [], onRemove,
 }: FeatureInjectionBarProps): JSX.Element {
   const [options, setOptions] = useState<CatalogOption[]>([]);
   const [selected, setSelected] = useState<string>("");
@@ -74,19 +79,50 @@ export function FeatureInjectionBar({
     onApply(injection);
   }
 
+  function removeOne(name: string) {
+    let popped: AppliedInjection | null = null;
+    for (let i = local.length - 1; i >= 0; i--) {
+      if (local[i].name === name) { popped = local[i]; break; }
+    }
+    if (!popped) return;
+    setLocal((prev) => {
+      const next = [...prev];
+      for (let i = next.length - 1; i >= 0; i--) {
+        if (next[i].name === name) { next.splice(i, 1); break; }
+      }
+      return next;
+    });
+    if (onRemove) onRemove(popped);
+  }
+
+  const counts = new Map<string, { count: number; paper_ref: string }>();
+  for (const a of local) {
+    const prev = counts.get(a.name);
+    counts.set(a.name, {
+      count: (prev?.count ?? 0) + 1,
+      paper_ref: a.paper_ref,
+    });
+  }
+  const chipEntries = Array.from(counts.entries());
+
   return (
     <div data-testid="feature-injection-bar"
          style={{ display: "flex", alignItems: "center", gap: 6,
-                  padding: 6, background: "#fefce8",
-                  border: "1px solid #fde047", borderRadius: 4,
-                  fontSize: 12 }}>
-      <strong style={{ color: "#0f172a", marginRight: 2 }}>Inject</strong>
-      <label style={{ display: "flex", alignItems: "center", gap: 4, color: "#0f172a" }}>
+                  padding: "4px 8px", background: T.surface,
+                  borderBottom: `1px solid ${T.border}`,
+                  fontSize: 12, fontFamily: T.font, color: T.text }}>
+      <strong style={{ color: T.accent, marginRight: 2 }}>Inject</strong>
+      <label style={{ display: "flex", alignItems: "center", gap: 4, color: T.textSecondary }}>
         <select
           data-testid="feature-injection-dropdown"
           value={selected}
           onChange={(e) => setSelected(e.target.value)}
           disabled={options.length === 0}
+          style={{
+            color: T.text,
+            background: T.surface3,
+            border: `1px solid ${T.border}`,
+          }}
         >
           {options.length === 0 && <option value="">…</option>}
           {options.map((o) => (
@@ -96,23 +132,54 @@ export function FeatureInjectionBar({
       </label>
       {selected && (
         <span data-testid="feature-injection-summary"
-              style={{ color: "#334155", fontWeight: 500 }}>
+              style={{ color: T.textSecondary, fontWeight: 500 }}>
           {options.find((o) => o.name === selected)?.summary ?? ""}
         </span>
       )}
       <button
         data-testid="feature-injection-apply"
         onClick={apply}
         disabled={!selected}
-        style={{ padding: "2px 8px" }}>
+        style={{
+          padding: "2px 8px",
+          background: T.surface3,
+          color: T.text,
+          border: `1px solid ${T.border}`,
+        }}>
         Apply
       </button>
       {err && <span data-testid="feature-injection-error"
-                    style={{ color: "#b91c1c" }}>{err}</span>}
+                    style={{ color: T.danger }}>{err}</span>}
       <span data-testid="feature-injection-applied-list"
-            style={{ color: "#374151", marginLeft: "auto" }}>
-        {local.length === 0 ? "—" :
-          local.map((a) => a.name).join(", ")}
+            style={{ display: "inline-flex", flexWrap: "wrap", gap: 4,
+                     marginLeft: "auto", alignItems: "center" }}>
+        {chipEntries.length === 0 ? (
+          <span style={{ color: T.textMuted }}>—</span>
+        ) : chipEntries.map(([name, { count }]) => (
+          <span key={name}
+                data-testid={`feature-injection-chip-${name}`}
+                style={{ display: "inline-flex", alignItems: "center",
+                         gap: 4, padding: "1px 4px 1px 6px",
+                         background: T.surface3,
+                         border: `1px solid ${T.border}`,
+                         borderRadius: 10, fontSize: 11, color: T.text }}>
+            <span>{name}</span>
+            {count > 1 && (
+              <span data-testid={`feature-injection-chip-${name}-count`}
+                    style={{ color: T.textSecondary, fontWeight: 600 }}>
+                ×{count}
+              </span>
+            )}
+            <button data-testid={`feature-injection-chip-${name}-remove`}
+                    onClick={() => removeOne(name)}
+                    title={count > 1 ? `remove one ${name}` : `remove ${name}`}
+                    style={{ background: "transparent", border: "none",
+                             color: T.textSecondary, cursor: "pointer",
+                             fontSize: 12, lineHeight: 1, padding: "0 2px" }}>
+              ×
+            </button>
+          </span>
+        ))}
       </span>
     </div>
   );
diff --git a/vbgui/tests/FeatureInjectionBar.test.tsx b/vbgui/tests/FeatureInjectionBar.test.tsx
@@ -107,6 +107,72 @@ describe("V8-R08 FeatureInjectionBar", () => {
     });
   });
 
+  it("applying the same option N times renders ONE chip with ×N count " +
+     "(UX#1: no more comma-joined mtp_weighted,mtp_weighted,...)", async () => {
+    const { rpc } = makeFakeRpc({ "catalog.list_options": CATALOG_OK });
+    render(<FeatureInjectionBar rpc={rpc} onApply={() => {}} />);
+    await waitFor(() => {
+      expect(screen.getByTestId("feature-injection-dropdown")).toBeDefined();
+    });
+    const applyBtn = screen.getByTestId("feature-injection-apply");
+    fireEvent.click(applyBtn);
+    fireEvent.click(applyBtn);
+    fireEvent.click(applyBtn);
+    fireEvent.click(applyBtn);
+
+    // ONE chip, not four.
+    expect(screen.getAllByTestId("feature-injection-chip-mtp_weighted"))
+      .toHaveLength(1);
+    // With ×4 count badge.
+    expect(screen.getByTestId("feature-injection-chip-mtp_weighted-count")
+      .textContent).toBe("×4");
+    // Applied list should NOT contain the literal comma-joined string
+    // "mtp_weighted, mtp_weighted, mtp_weighted, mtp_weighted".
+    const list = screen.getByTestId("feature-injection-applied-list");
+    expect(list.textContent ?? "").not.toContain(
+      "mtp_weighted, mtp_weighted");
+  });
+
+  it("chip × button calls onRemove and pops one instance", async () => {
+    const { rpc } = makeFakeRpc({ "catalog.list_options": CATALOG_OK });
+    const onRemove = vi.fn();
+    render(<FeatureInjectionBar rpc={rpc} onApply={() => {}}
+                                 onRemove={onRemove} />);
+    await waitFor(() => {
+      expect(screen.getByTestId("feature-injection-dropdown")).toBeDefined();
+    });
+    const applyBtn = screen.getByTestId("feature-injection-apply");
+    fireEvent.click(applyBtn);
+    fireEvent.click(applyBtn);
+    fireEvent.click(applyBtn);
+    expect(screen.getByTestId("feature-injection-chip-mtp_weighted-count")
+      .textContent).toBe("×3");
+
+    fireEvent.click(screen.getByTestId(
+      "feature-injection-chip-mtp_weighted-remove"));
+    expect(onRemove).toHaveBeenCalledTimes(1);
+    expect(onRemove.mock.calls[0][0]).toEqual({
+      name: "mtp_weighted",
+      paper_ref: "rewriter:MTPRewriter",
+    });
+    // Count drops to ×2.
+    expect(screen.getByTestId("feature-injection-chip-mtp_weighted-count")
+      .textContent).toBe("×2");
+  });
+
+  it("single-instance chip renders without a count badge", async () => {
+    const { rpc } = makeFakeRpc({ "catalog.list_options": CATALOG_OK });
+    render(<FeatureInjectionBar rpc={rpc} onApply={() => {}} />);
+    await waitFor(() => {
+      expect(screen.getByTestId("feature-injection-dropdown")).toBeDefined();
+    });
+    fireEvent.click(screen.getByTestId("feature-injection-apply"));
+    expect(screen.getByTestId("feature-injection-chip-mtp_weighted"))
+      .toBeDefined();
+    expect(screen.queryByTestId(
+      "feature-injection-chip-mtp_weighted-count")).toBeNull();
+  });
+
   it("renders error banner on RPC failure", async () => {
     const { rpc } = makeFakeRpc({
       "catalog.list_options": new Error("catalog down"),
