Skip to content

refactor!: prune modules unreachable from any known consumer #55

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheGreatAxios merged 2 commits into from
Sep 21, 2026
Merged

refactor!: prune modules unreachable from any known consumer #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9a4696
refactor!: prune modules unreachable from any known consumer
TheGreatAxios Sep 21, 2026
7bd2eaf
refactor(lib): drop step-graph helpers left in chart-geometry
TheGreatAxios Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 40 additions & 104 deletions ARCHITECTURE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ per source file, no bundling — and `sideEffects` is declared CSS-only. The roo
re-exports only, so importing from the root and importing by subpath both bundle just
what you used. They are not guaranteed byte-for-byte identical — a bundler may order or
name things differently depending on the entry — but neither drags in a component you did
not reference. One module is deliberately excluded from the root entry; see below.
not reference.

**Abstraction has to earn its place.** No config indirection, no speculative extension
points. There is no CLI, no component generator and no docs site, and that is deliberate.
Expand Down Expand Up @@ -47,73 +47,12 @@ its own boundary (see README). Baking one framework's convention into a hundred
published files would make correctness depend on a build step no plain-React consumer
runs, and hide the boundary where the consumer cannot see it.

## The `DataPort` seam
## No fetch, ever

Components in this package **never fetch**, and never import a server-side package. A
component that fetches has chosen the consumer's data layer for them; a component that
imports a server package drags a database tree into a browser bundle.

Data arrives one of two ways: as props, or through a `DataPort`. That is the
README contract — components take data through a `DataPort` (or none). "None"
is a props-only surface; `DataPortProvider` is required only for components
that call `useDataPort`.

```ts
type DataPort = {
readonly useCollection: <T>(request: CollectionRequest<T>) => CollectionResult<T>;
};
```

A component declares *what* it needs — a stable cache key plus a `fetch` thunk that takes
`{ signal, offset, pageSize }` and returns `{ items, nextOffset }` — and learns nothing
about how the result is cached, deduped or revalidated. The port is resolved from React
context (`DataPortProvider` / `useDataPort`), and `useDataPort` throws with a wiring
message rather than returning `null`.

Three details in `CollectionResult` are load-bearing:

- **`isLoading` and `isFetching` are separate.** `isLoading` means there is nothing to
show yet; `isFetching` means a request is in flight, including a background refetch
over rows already on screen. Only `isFetching` is an honest input to `aria-busy`.
- **Pagination costs one field, not a second code path.** A source that returns
everything at once reports `nextOffset: null`. There is no paginated variant of any
component.
- **Collections only.** There is deliberately no `useRecord` and no `useMutation` — a
mutation shape invented before its first consumer is a shape that will be wrong.

`createTanstackDataPort()` (`lib/tanstack-data-port`) implements `DataPort` over TanStack
Query's infinite-query API. It is a separate module on purpose: the seam is the type, the
adapter is one implementation, and that separation is what lets `@tanstack/react-query`
stay an **optional** peer. A consumer on SWR, a websocket cache or a fixture port swaps
the provider value and changes no component.

The adapter is therefore the one public module excluded from the root barrel
(`BARREL_EXCLUDED` in `scripts/generate-exports.mjs`). `src/index.ts` is a single module:
a bundler tree-shakes *bindings* out of it, but still resolves every module it re-exports,
so re-exporting the adapter would make `import { Button } from "@corbits/react-ui"` fail
outright for anyone who did not install the optional peer. This is bundler-dependent —
Node and Vite fail, Turbopack compiles it — so `dep-guard` walks the relative import graph
from `src/index.ts` and fails if anything reachable from it imports an optional peer.

## `use-collection-state`

Every collection surface — a table, a list, a grid, a step rail — has the same four
states. `useCollectionState(request)` collapses a `CollectionResult` into exactly that
discriminated union and **renders nothing**.

```ts
type CollectionState<T> =
| { status: "loading" }
| { status: "error"; error: Error }
| { status: "empty" }
| { status: "ready"; items: readonly T[] };
```

The ordering is the whole machine: `isLoading` wins, then `error`, then an empty `items`,
then ready. Getting it wrong is how a surface renders "no results" during a first load, or
an empty state over a failed request. It renders nothing because the markup belongs to the
surface; `isFetching`, `refetch`, `hasNextPage` and `fetchNextPage` pass through unchanged.

imports a server package drags a database tree into a browser bundle. Data arrives as
props — the host owns its own data layer, caching and pagination.
## `CommandPalette`

`CommandPalette` is a global-search overlay with no idea what it is searching. It takes
Expand Down Expand Up @@ -215,30 +154,26 @@ validator yourself.

## Pieces, then one wrapper

A composite surface — an inspector, a registry list, a step diagram — is built as small
stateless pieces plus at most one composition wrapper. Each piece is its own file and its
own export, takes everything through props, holds no state of its own, and renders on its
own (`LiveRunHeader`, `LiveRunBanner`, `StepList`, `InspectorKv`, `InspectorPanelTitle`).
The wrapper (`LiveRunInspector`) contains no markup the pieces do not already carry: it
only decides which pieces render and in what order, so replacing one piece never means
forking the wrapper. `WorkflowStatusBadge` is the same idea one level down — a `Badge`
carrying a `StatusDot`, not a bespoke "status chip" component, because `Badge` and
`StatusDot` already exist and composing them is cheaper than a third thing that does
what the first two already do together.
A composite surface — an inspector, a step rail, a chat thread — is built as small
stateless pieces plus at most one composition wrapper. Each piece takes everything
through props, holds no state of its own, and renders on its own (`LiveRunBanner`,
`StepList`, `InspectorKv`, `InspectorPanelTitle`, `SidebarPanelBody`). A wrapper
contains no markup the pieces do not already carry: it only decides which pieces
render and in what order, so replacing one piece never means forking the wrapper.

Behaviour and arithmetic split out the same way: a reusable behaviour becomes a headless
hook in `src/hooks/` (`use-scroll-current-into-view` renders nothing and returns a ref),
and layout math becomes plain functions in `src/lib/` (`step-graph-layout`, like
`chart-geometry`, is numbers in and numbers out, checkable without rendering anything).
and layout math becomes plain functions in `src/lib/` (`chart-geometry` is numbers in
and numbers out, checkable without rendering anything).

## Layout

| | |
| --- | --- |
| `src/ui/` | Components. Primitives, collection surfaces, shells, and the domain families. |
| `src/lib/` | Non-component source: `utils`, the `DataPort` seam and its adapter, the per-domain shapes, and the internal chart palette and geometry. |
| `src/hooks/` | 15 headless hooks: `use-collection-state`, `use-anchored-scroll`, `use-chat-dock`, `use-command-palette-navigation`, `use-controllable-state`, `use-delayed-autofocus`, `use-dismissable-popover`, `use-flip-transition`, `use-focus-trap`, `use-prefers-reduced-motion`, `use-render-rail`, `use-resizable-rail`, `use-scroll-current-into-view`, `use-sidebar-panel`, `use-view-mode`. |
| `src/blocks/` | Multi-file compositions (`login`, `access-notice`, `canvas-host`). |
| `src/lib/` | Non-component source: `utils`, the per-domain shapes, and the internal chart palette and geometry. |
| `src/hooks/` | 7 headless hooks: `use-command-palette-navigation`, `use-controllable-state`, `use-delayed-autofocus`, `use-dismissable-popover`, `use-list-selection`, `use-prefers-reduced-motion`, `use-scroll-current-into-view`. |
| `src/blocks/` | Multi-file compositions (`login`). |
| `src/theme.css` | Tokens, keyframes, base layer. The only CSS source. |
| `src/styles.css` | Two `@import`s. The entry Tailwind compiles into `dist/styles.css`. |
| `src/index.ts` | The root barrel. **Generated** — do not edit. |
Expand All @@ -255,6 +190,13 @@ line-by-line review of every file; it is a targeted grep-and-read over the
patterns below, cross-checked against what similar components in the
library already do.

This is a historical record of that pass. Several components it names —
`TenantSelector`, `ThreadSwitcher`, `SubagentDock`, `OnboardingTour`,
`DitherBackground`, `SidebarRail`, `StepSidebar`, `LiveRunInspector` and
others — were removed later, when a consumer-usage audit pruned every module
no known consumer imports. The findings and the standing rules they produced
still apply to what remains.

Every surveyed module is re-exported from the package root (`src/index.ts`),
so "public surface" doesn't distinguish findings the way it might in an app —
everything here is public. Blast radius instead counts how many other
Expand Down Expand Up @@ -307,9 +249,9 @@ Audited every array-of-object prop in `src/ui`/`src/blocks`
`kind-card-grid`, `progress-checklist`, `step-list`, `horizontal-stepper`,
and others). **No findings.** Every one of these is a collection-rendering
component — a table row set, a step rail, a filter list — consistent with
the `DataPort`/`use-collection-state` architecture above, where components
the props-only data flow above, where components
render data the host doesn't have pre-built React elements for (it has
domain objects: a `Tenant`, a `WorkflowStep`, a `FilterSpec`). Slots are
domain objects: a `WorkflowStep`, a `FilterSpec`). Slots are
used elsewhere in the library exactly where the content genuinely is
caller-arbitrary markup (`NotificationsBell`'s `children`, `AuthLayout`'s
`panel`). Swapping a config-array prop for children in a collection
Expand Down Expand Up @@ -401,10 +343,9 @@ already in the tree.
piece of state answers "is this open/collapsed/expanded" and a reasonable
host might want to read or drive it — closing every other panel when one
opens, restoring a collapsed sidebar from a saved preference — it's a prop
pair: `open` + `onOpenChange`, or `collapsed` + `onToggle`. `Sidebar`,
`StepSidebar` and `CanvasHost` set this precedent; `Dialog` and
`CommandPalette` apply it to popups; `NotificationsBell`, `TenantSelector`,
`ThreadSwitcher`, `ToolBlock` and `ToolNarrative` now do too, all
pair: `open` + `onOpenChange`, or `collapsed` + `onToggle`. `Sidebar` sets
this precedent; `Dialog` and `CommandPalette` apply it to popups;
`NotificationsBell`, `ToolBlock` and `ToolNarrative` do too, all
uncontrolled-by-default so adding the pair is never a breaking change. The
exception is state a parent has no legitimate reason to want: `ActivityBlock`
stays a native `<details>`, and a text-truncation toggle stays local. If
Expand All @@ -424,9 +365,8 @@ component whose job is to render caller-supplied content it has no opinion
about (`NotificationsBell`'s `children`, `AuthLayout`'s `panel`) takes
`children` or a named slot. A component whose job is to render a
*collection* of the library's own domain shapes (a list of `WorkflowStep`, a
list of `Tenant`) takes that collection as a data prop, the same way
`DataPort` and `use-collection-state` already assume — that's what makes a
table, a list and a step rail interchangeable consumers of one
list of chat messages) takes that collection as a data prop — that's what
makes a table, a list and a step rail interchangeable consumers of one
loading/empty/error contract. Don't reach for a config-object prop as a
substitute for either of these; if you're tempted to add one, first check
whether the content is actually caller-arbitrary (make it `children`) or
Expand All @@ -436,12 +376,12 @@ shape).
**Non-trivial imperative behavior lives in a headless hook, not the
component body.** Event listeners, focus management, measurement, timers,
keyboard handling — if two components would otherwise duplicate the same
`useEffect`, that's the hook boundary. `use-focus-trap`,
`use-command-palette-navigation`, `use-resizable-rail`,
`use-dismissable-popover` and `use-prefers-reduced-motion` are the existing
examples: each owns exactly the imperative part and returns refs/values, no
markup. Before writing a second `addEventListener` pair that looks like one
already in the tree, grep `src/hooks` first.
`useEffect`, that's the hook boundary. `use-command-palette-navigation`,
`use-dismissable-popover`, `use-prefers-reduced-motion` and
`use-controllable-state` are the existing examples: each owns exactly the
imperative part and returns refs/values, no markup. Before writing a second
`addEventListener` pair that looks like one already in the tree, grep
`src/hooks` first.

**Every color comes from a token, with no unconditional literal fallback.**
`src/theme.css` is the only place a hex, rgb or named color belongs. A
Expand All @@ -456,23 +396,19 @@ literal is by construction correct in at most one theme.
global CSS rule handles every `transition-*`/`animate-*` utility for free —
don't re-implement that check for a component using only CSS transitions.
The rule doesn't reach `requestAnimationFrame` loops or imperative calls
like `scrollIntoView`; those call `usePrefersReducedMotion()` (or, for
something that needs its own live `MediaQueryList` listener mid-loop,
follow `DitherBackground`'s pattern) rather than a one-shot
`matchMedia(...).matches` read that goes stale the moment the OS setting
changes.
like `scrollIntoView`; those call `usePrefersReducedMotion()` rather than a
one-shot `matchMedia(...).matches` read that goes stale the moment the OS
setting changes.

## Known limits

- **`DataPort` covers collections only.** Single-record reads and mutations are absent by
decision, not by oversight.
- **Tests exist but are partial.** `bun test` runs 338 tests co-located with their
subjects (`src/**/*.test.tsx`), plus shared harnesses under `src/test/` — the
measurable half of appearance (contrast, in both modes) is gated in the build,
and a growing slice of behaviour is now covered by these, but most components
still have no test.
- **A rendering gallery exists but is partial.** Ladle (`bun run stories`, `bun run
stories:build`) renders 52 stories under `stories/`; most of the ~150 components have
still have no test.- **A rendering gallery exists but is partial.** Ladle (`bun run stories`, `bun run
stories:build`) renders 42 stories under `stories/`; most of the ~90 components have
none, so visual review of an uncovered component still means rendering the package
inside a consumer app.
- **The prebuilt stylesheet restyles the consuming page** — it carries Tailwind's
Expand Down
37 changes: 31 additions & 6 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,41 @@ contract — modules that are not exported are internal and may change in any re

### Breaking changes since 0.1.0

- **84 modules removed — every module no known consumer imports.** A usage audit across
the workbench, portal, scout and solutions-builder codebases showed these surfaces were
unreachable from any consumer, so they were deleted rather than carried as semver
promises. The removed clusters: the entire `DataPort` seam (`lib/data-port`,
`lib/tanstack-data-port`, `hooks/use-collection-state`) and its dependents
(`data-table`, `thread-list`, `command-queue`); the now-inbox surfaces (`mail-detail`,
`triage-pane`, `context-strip`, `activity-timeline`, `moment-walker`, `actor-summary`);
all scheduling (`schedule-*`, `run-once-flow`,
`recurrence-input`); the workflow registry (`workflow-registry-*`, `workflow-catalog`,
`workflow-dock`, `workflow-status-badge`); step-graph visualization (`step-graph`,
`step-graph-node`, `step-sidebar`, `step-primitives`, `lib/step-graph-layout`);
`subagent-dock`, `chat-dock` and `message-list`; the canvas/artifact-host surfaces
(`access-notice`, `canvas-host`, `canvas-host-chrome`, `generative-block-view`,
`artifact-detail`, `artifact-gallery`, `add-artifact-dialog`); and the unused
primitives and screens (`dial`, `pagination`, `radio-group`, `pulsing-ring`,
`dashboard`, `dashboard-section`, `whats-new`, `onboarding-tour`,
`reconnecting-overlay`, `stat-tile`, `delta-badge`, `count-table`, `heatmap`,
`kind-picker`, `toggle-list`, `managed-list`, `tenant-selector`, `thread-switcher`,
`section-nav`, `sidebar-rail`, `sidebar-section`, `read-block`, `render-rail`,
`failed-run-notice`, `live-run-header`, `live-run-inspector`, `time-range-control`,
`library-search-input`, `catalog-glyph`, `command`, `lib/command-registry`,
`dither-background`, and the hooks that only served them). `agent-turn` and
`activity-block` stayed — `chat-thread` uses them. The package is now 113
public modules. **`@tanstack/react-query` is no longer a peer dependency** —
nothing in the package imports it.
- **`AuthLayout` no longer defaults its decorative panel to `DitherCanvas`.** The panel is
now an explicit `panel` slot with no fallback — pass `<DitherCanvas />`,
`<DitherBackground src={...} />`, or any other content. A caller that omitted `panel` to
get the dither canvas for free now gets a bare panel instead.
now an explicit `panel` slot with no fallback — pass `<DitherCanvas />` or any other
content. A caller that omitted `panel` to get the dither canvas for free now gets a
bare panel instead. (`DitherBackground` was among the removed modules.)
- **`ui/command` is removed.** `Command` was a second, weaker inline command surface next
to `CommandPaletteInline` — index-based keyboard navigation (which loses the highlighted
row when the list re-sorts under a keystroke) and self-filtering, where the palette
family deliberately leaves matching to the caller. `CommandAction` moves to
`lib/command-registry`, its only consumer; `commandMatches` is gone with the component.
Root-barrel imports of `CommandAction` still work; the `./ui/command` subpath is gone.
family deliberately leaves matching to the caller. `commandMatches` is gone
with the component. The `./ui/command` subpath is gone. `lib/command-registry`
(and `CommandAction`) is also in the unconsumed prune above.

## [0.1.0] - 2026-09-17

Expand Down
20 changes: 9 additions & 11 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ bun install
bun run build
```

There is no database and no backend. If a change needs one, it belongs behind the
`DataPort` seam, not in a component.
There is no database and no backend. If a change needs one, it belongs in the
consumer's data layer, not in a component — data arrives as props.

## Gates

Expand Down Expand Up @@ -48,25 +48,23 @@ the build gate does not cover them. Record the result in the change.

## The dep-guard

`scripts/dep-guard.mjs` walks every `.ts` / `.tsx` under `src/` and enforces three rules.
`scripts/dep-guard.mjs` walks every `.ts` / `.tsx` under `src/` and enforces two rules.
The forbidden strings are named here deliberately — a guard whose forbidden string is a
secret is a guard nobody can comply with.

1. **No `@workbench/*`.** A published file that imports an unpublished scope is
uninstallable for everyone outside the project that defines it, and the failure lands
in the *consumer's* install, where they cannot fix it.
2. **Only `lib/tanstack-data-port.ts` imports `@tanstack/react-query`.** That is what lets
the query library be an *optional* peer dependency.
3. **Nothing reachable from the root barrel imports an optional peer.** Rule 2 alone does
not cover this: re-exporting the adapter from `src/index.ts` keeps rule 2 green while
making the optional peer mandatory for every root import.
2. **Nothing reachable from the root barrel imports an optional peer.** Re-exporting a
module that statically imports an optional peer makes that peer mandatory for every
root import, so such a module must be kept out of the barrel (`BARREL_EXCLUDED`).

The same rule, not machine-checked, applies to server-side packages: a component must
never import one. They carry database drivers, and pulling one into a browser bundle is a
category error. Data reaches a component through props or a `DataPort`.
category error. Data reaches a component through props.

Wanting to relax any of this is the signal to widen the `DataPort` or add a prop — never
to import your way around it.
Wanting to relax any of this is the signal to add a prop — never to import your way
around it.

## Adding a component

Expand Down
Loading
Loading