From 2303ff4c78f162c73590a9e5fcb7d63b31d4bdcd Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Mon, 24 Aug 2026 16:34:16 -0700 Subject: [PATCH 1/3] chore(frontend): remove the icon library nothing imports any more The annotation workspace was the last place drawing `lucide-react`, and it moved to Tabler two phases ago. Nothing in the repository imports it now - not the product, not the annotator, not the demo, not a test, not the benchmark - so the two manifests that still declared it were describing a dependency the frontend had stopped having. The lockfile is resolved from scratch against the new manifests and a clean frozen install runs from an empty tree, so this is not a manifest edit hoping the lock agrees with it. The frontend dependency bump (#768) raises this package's version on the same two lines. It will want a rebase that drops the entry rather than a merge; the other four bumps it carries are unaffected. --- frontend/app/package.json | 1 - frontend/ui-core/package.json | 1 - pnpm-lock.yaml | 15 --------------- 3 files changed, 17 deletions(-) diff --git a/frontend/app/package.json b/frontend/app/package.json index b5dabea8..cdfa9a2b 100644 --- a/frontend/app/package.json +++ b/frontend/app/package.json @@ -20,7 +20,6 @@ "@tabler/icons-react": "^3.46.0", "@visionset/annotator": "workspace:*", "@visionset/ui-core": "workspace:*", - "lucide-react": "^1.32.0", "react": "^19.0.0", "react-dom": "^19.0.0", "react-router": "^8.3.0" diff --git a/frontend/ui-core/package.json b/frontend/ui-core/package.json index 9dcd4a77..53e67df0 100644 --- a/frontend/ui-core/package.json +++ b/frontend/ui-core/package.json @@ -59,7 +59,6 @@ "@visionset/annotator": "workspace:*", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", - "lucide-react": "^1.32.0", "openapi-fetch": "^0.17.0", "shadcn": "^4.18.0", "sonner": "^2.0.8", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f06bb428..0190b8da 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -59,9 +59,6 @@ importers: '@visionset/ui-core': specifier: workspace:* version: link:../ui-core - lucide-react: - specifier: ^1.32.0 - version: 1.32.0(react@19.2.8) react: specifier: ^19.0.0 version: 19.2.8 @@ -174,9 +171,6 @@ importers: clsx: specifier: ^2.1.1 version: 2.1.1 - lucide-react: - specifier: ^1.32.0 - version: 1.32.0(react@19.2.8) openapi-fetch: specifier: ^0.17.0 version: 0.17.0 @@ -2511,11 +2505,6 @@ packages: lru-cache@5.1.1: resolution: {integrity: sha512-KpNARQA3Iwv+jTA0utUVVbrh+Jlrr1Fv0e56GGzAFOXN7dk/FviaDW8LHmK52DlcH4WP2n6gI8vN1aesBFgo9w==} - lucide-react@1.32.0: - resolution: {integrity: sha512-txX56hMFnRxPi1f9/nH69YN8uvAO6a7Y1KSWKjCDAtdD9+soEgmWuCt6iRm1pkxUZo2+YntSdsE1L6bIuKoY8Q==} - peerDependencies: - react: ^16.5.1 || ^17.0.0 || ^18.0.0 || ^19.0.0 - lz-string@1.5.0: resolution: {integrity: sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==} hasBin: true @@ -5533,10 +5522,6 @@ snapshots: dependencies: yallist: 3.1.1 - lucide-react@1.32.0(react@19.2.8): - dependencies: - react: 19.2.8 - lz-string@1.5.0: {} magic-string@0.30.21: From 43ac7fd641e432a81430e956e548f7a2ec3dfbf6 Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Mon, 24 Aug 2026 16:34:35 -0700 Subject: [PATCH 2/3] test(frontend): pin the icon set from the manifest as well as the imports The gate added when the workspace migrated scanned that one directory for the old package's name. That was the right shape while the dependency was still declared and a stray import would resolve, build and render. With the dependency gone such an import fails outright, so the loud half no longer needs guarding - and the quiet half now does: a manifest that declares the package again restores the whole problem, and nothing else in the repository would notice. So the invariant moves beside the other design gates and asserts both halves over the whole tracked tree rather than one directory. Both were checked by breaking them, separately: a manifest that declares the package fails it, and a source that imports it fails it naming the file. `iconSet.test.ts` goes with it, subsumed - the new scan reads the directory it watched, and two tests for one rule is one of them going stale unnoticed. --- .../ui-core/src/annotator/iconSet.test.ts | 48 ------------------- tests/scripts/design_tokens.test.mjs | 45 +++++++++++++++++ 2 files changed, 45 insertions(+), 48 deletions(-) delete mode 100644 frontend/ui-core/src/annotator/iconSet.test.ts diff --git a/frontend/ui-core/src/annotator/iconSet.test.ts b/frontend/ui-core/src/annotator/iconSet.test.ts deleted file mode 100644 index f6c4ca26..00000000 --- a/frontend/ui-core/src/annotator/iconSet.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** - * The annotation workspace draws Tabler, and nothing else. - * - * `DESIGN.md`'s icon rule names this directory as the last place `lucide-react` - * survived: the primitives and the screens were converged first, and the - * annotator's own glyphs came last. Once migrated, the interesting failure is not - * a wrong glyph but a *reintroduction* — a new file importing the old set because - * a neighbouring branch still did, or because an editor auto-imported it while - * both packages sat in `node_modules`. Nothing else in the repository would - * notice: the dependency stays declared until the annotator's consumers drop it, - * so a stray import resolves, builds, and renders. - * - * The scan is over the whole directory rather than a list of the eleven files - * that were migrated. A list would pass for a twelfth file nobody added to it, - * which is the failure mode a gate exists to prevent, and the emptiness - * assertion below is what stops the scan from proving nothing if the glob ever - * stops matching. - */ - -import { readdirSync, readFileSync } from "node:fs"; -import { resolve } from "node:path"; -import { describe, expect, it } from "vitest"; - -/** - * `import.meta.url` is an `http://localhost/` URL under jsdom, so the path - * resolves from the package root vitest runs in — the reason `ProjectNav`'s own - * Tabler assertion spells it this way too. - */ -const ANNOTATOR = resolve(process.cwd(), "src/annotator"); - -/** Every production module here — the tests themselves are not the contract. */ -function productionSources(): readonly string[] { - return readdirSync(ANNOTATOR) - .filter((name) => /\.tsx?$/.test(name) && !/\.test\.tsx?$/.test(name)) - .sort(); -} - -describe("the annotation workspace's icon set", () => { - it("draws Tabler only, in every production module", () => { - const sources = productionSources(); - expect(sources.length).toBeGreaterThan(0); - - const offenders = sources.filter((name) => - readFileSync(resolve(ANNOTATOR, name), "utf8").includes("lucide-react"), - ); - expect(offenders).toEqual([]); - }); -}); diff --git a/tests/scripts/design_tokens.test.mjs b/tests/scripts/design_tokens.test.mjs index 142a523f..1fcf05ff 100644 --- a/tests/scripts/design_tokens.test.mjs +++ b/tests/scripts/design_tokens.test.mjs @@ -318,6 +318,51 @@ test("components.json holds the schema-supported preset fields, and no others", assert.equal(config.tailwind.css, "src/styles.css"); }); +/** + * Tabler is the icon set, and the only one — the final state of a migration that + * ran through the primitives, the screens and the annotation workspace in turn. + * + * The interesting failure is a *return*, not the original debt: an editor + * auto-import, or a branch that predates the migration coming back through a + * merge. Now that no manifest declares the old package such an import fails to + * resolve, which is the loud half. The quiet half is the manifest — a dependency + * added back "because something imported it" restores the whole problem without + * anything else noticing, so both halves are asserted here. + * + * Assembled from fragments so this file never contains the package's name as a + * contiguous string, and a repository-wide sweep for it never mistakes its own + * guard for a lingering usage — the trick `HEX` and `RETIRED_DECLARATIONS` above + * already use. + */ +const RETIRED_ICON_PACKAGE = ["lucide", "react"].join("-"); + +test("no package declares a second icon set, and no source imports one", () => { + const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); + assert.equal(listed.status, 0, `git ls-files failed: ${listed.stderr}`); + const tracked = listed.stdout.split("\0").filter(Boolean); + + const manifests = tracked.filter((name) => /(?:^|\/)package\.json$/.test(name)); + assert.ok(manifests.length > 0, "no manifests were read, so this proves nothing"); + const declaring = manifests.filter((name) => + readFileSync(path.join(REPO, name), "utf8").includes(`"${RETIRED_ICON_PACKAGE}"`), + ); + assert.deepEqual( + declaring, + [], + "@tabler/icons-react is the frontend's icon set. A second one is a decision for " + + `DESIGN.md, not a dependency:\n${declaring.join("\n")}`, + ); + + const sources = tracked.filter((name) => SOURCE.test(name) && !GENERATED.test(name)); + assert.ok(sources.length > 0, "no frontend sources were read, so this proves nothing"); + const importing = sources.filter((name) => + new RegExp(String.raw`(?:from|require\()\s*["']${RETIRED_ICON_PACKAGE}["']`).test( + readFileSync(path.join(REPO, name), "utf8"), + ), + ); + assert.deepEqual(importing, [], `these draw from the retired icon set:\n${importing.join("\n")}`); +}); + test("the tokens have exactly one home, and it is the stylesheet", () => { const listed = spawnSync("git", ["ls-files", "-z"], { cwd: REPO, encoding: "utf8" }); assert.equal(listed.status, 0, `git ls-files failed: ${listed.stderr}`); From 5dcc7bb2e539ba72183305cb2380a0966808454b Mon Sep 17 00:00:00 2001 From: YaelAnaya Date: Mon, 24 Aug 2026 16:34:35 -0700 Subject: [PATCH 3/3] docs(frontend): the icon set reads as settled rather than mid-migration Six places still described a migration that had finished. `DESIGN.md` and the frontend setup skill both said Lucide survived in the annotation workspace, and both were wrong the day they were written: the change that wrote them is the one that migrated that directory. The architecture note repeated it, the `ui-core` barrel still credited "Radix + lucide only (decision H)", and the demo's tool strip explained its hand-drawn glyphs by citing a pin `DESIGN.md` dropped when the preset landed. Each now says what is true, and `DESIGN.md` points at the gate rather than at a directory, so the claim cannot go stale the same way twice. The demo's note says less than the one it replaces, deliberately. The tempting explanation - that the demo runs without `@visionset/ui-core` - is also false: it imports `AnnotatorPanel` and the theme helpers from there. What holds is narrower, and is what the note now says: this strip imports only the engine. --- .agents/skills/frontend/nodejs-setup/SKILL.md | 4 ++-- DESIGN.md | 11 +++++------ docs/content/architecture/frontend/ui-core.md | 2 +- frontend/app/src/demo/ToolStrip.tsx | 14 ++++++-------- frontend/ui-core/src/index.ts | 2 +- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/.agents/skills/frontend/nodejs-setup/SKILL.md b/.agents/skills/frontend/nodejs-setup/SKILL.md index b6c20430..8c757377 100644 --- a/.agents/skills/frontend/nodejs-setup/SKILL.md +++ b/.agents/skills/frontend/nodejs-setup/SKILL.md @@ -64,8 +64,8 @@ pnpm add -w -D # root tooling only - `@visionset/annotator` keeps `react` as an **optional peer dependency** — never promote it to a hard dependency; the core must stay usable without React. - `@visionset/ui-core` UI primitives are **Radix behaviour + shadcn Nova styling, iconed with - Tabler**. Do not add another component library, and do not reach for `lucide-react` in new - code — it survives only in `src/annotator/`, as migration debt. + Tabler**. Do not add another component library, and do not add a second icon set: + `@tabler/icons-react` is the only one the frontend declares, and a gate refuses another. - `@visionset/app` is `private: true` — never published to npm; its bundle ships inside the Python wheel. diff --git a/DESIGN.md b/DESIGN.md index 936e5322..5b7f8e32 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -409,12 +409,11 @@ First-class, and part of every rule above rather than a section to satisfy after - **Ad-hoc geometry that fights Nova.** A control's height, padding, or radius is not a per-screen decision; reaching past the geometry table above for a bespoke size is a design decision to make in this document, not in a component diff. -- **Mixing icon sets in new code.** Tabler is the set, and across the primitives *and* - the screens it is already the only one: every icon `frontend/ui-core/src/primitives/` - and `frontend/ui-core/src/screens/` draws is `@tabler/icons-react`, and neither - directory imports `lucide-react` at all. Lucide survives in exactly one place — - `frontend/ui-core/src/annotator/` — and that is the whole of the remaining debt: a file - there still importing it is a file to migrate, not a precedent to follow. +- **Mixing icon sets in new code.** Tabler is the set, and now the only one: every icon + the primitives, the screens and the annotation workspace draw is + `@tabler/icons-react`, no package declares a second icon library, and + `tests/scripts/design_tokens.test.mjs` refuses one that reappears in a manifest or an + import. A second set is a decision to make in this document, not a dependency to add. - **Brand in a functional control.** Robomous coral is identity — the wordmark and its styleguide swatch, nothing else. A functional control reaching for `brand` is a semantic-colour violation regardless of how many other sites already use it correctly. diff --git a/docs/content/architecture/frontend/ui-core.md b/docs/content/architecture/frontend/ui-core.md index 8205df2d..f74fc618 100644 --- a/docs/content/architecture/frontend/ui-core.md +++ b/docs/content/architecture/frontend/ui-core.md @@ -106,7 +106,7 @@ visual-design rule. The current choices: | Concern | Choice | | --- | --- | | UI primitives | Radix (+ shadcn-style composition with `cva` and `cn`) - the open-code shadcn maintenance model is the direction: a primitive is VisionSet-owned source in `frontend/ui-core/src/primitives/`, edited directly, not a package dependency upgraded blindly | -| Icons | `@tabler/icons-react` is the set: the primitives and the screens draw nothing else. `lucide-react` remains only in `src/annotator/`, as migration debt | +| Icons | `@tabler/icons-react`, and nothing else: the primitives, the screens and the annotation workspace all draw from it, and no package declares a second icon library | | Styling | Tailwind v4, CSS-first `@theme`, on the shadcn preset `b3bXyyPdWj` - no `tailwind.config.js`, ever | | Toasts | sonner | | Component tests | vitest + jsdom + @testing-library/react | diff --git a/frontend/app/src/demo/ToolStrip.tsx b/frontend/app/src/demo/ToolStrip.tsx index b3b4ded1..0a71eb0e 100644 --- a/frontend/app/src/demo/ToolStrip.tsx +++ b/frontend/app/src/demo/ToolStrip.tsx @@ -32,14 +32,12 @@ * "what a product does about a missing tool" is a product decision it is not * the job of a demo to make. * - * ## The icons are hand-drawn, and that is a deferral rather than a preference + * ## The icons are hand-drawn * - * `DESIGN.md` pins lucide-react (MousePointer2 / Square / Spline) and the design - * system is the - * task that installs it. Adding a dependency to `@visionset/app` so a demo can - * draw three glyphs would put the choice in the wrong milestone, so these are - * three inline paths at lucide's 24-unit grid and 2px stroke. They are replaced, - * not extended, when the real icon set arrives. + * Three inline paths on a 24-unit grid at 2px stroke. This strip draws its own + * chrome from `./theme` and imports nothing but the engine, which is the point of + * it — the product's own geometry glyphs are `GeometryIcon`'s, and they are not + * what a demo of the headless engine should be exercising. */ import { drawableGeometries, hotkeyForClass } from "@visionset/annotator"; @@ -219,7 +217,7 @@ function IconButton({ ); } -/** lucide's grid and stroke, three paths of it. See the header note. */ +/** Three paths on the grid and stroke the header note describes. */ function ToolIcon({ tool }: { readonly tool: Tool }): JSX.Element { return (