refactor: split oversized modules into focused ones - #45
Open
AlexKempen wants to merge 23 commits into
Open
Conversation
Breaks up shared/types.ts, shared/api-models.ts, backend/app.ts, backend/auth.ts and frontend/queries.ts along their seams so the following move to a feature layout is a pure relocation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Replaces src/shared with ownership-based placement: the backend owns
the contract (DTOs, domain enums, configuration models) and the
frontend imports it, so there is no third top-level bucket.
backend/ app.ts assembles Hono; db/ holds the client and schema; lib/
holds the Onshape client and request plumbing; features/{auth,users,
library,configurations,thumbnails,build-checker,favorites,search} each
own their routes, storage and models.
frontend/ features/<feature>/{queries.ts,components/} with cross-cutting
helpers in lib/ and shared UI in components/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
The frontend reaches the backend's DTOs and domain models through @backend/* instead of counting ../ levels; imports within a side stay relative. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Every module that turns an Onshape response into what we store now lives in features/load: the parse-* modules (document contents, configurations, configuration records, vendors, fasten info), the per-group and per-insertable loaders, and the Workflows and job tracker that drive them. Two boundaries were untangled to make that a clean feature: - ThumbnailWorkflow moved to features/thumbnails, and the render/upload helpers moved out of its routes into store.ts. load and thumbnails previously imported each other in a cycle; the dependency is now one-way from thumbnails to load. - parse-fasten mixed parsing with insert-time query building. getFastenQuery moved to features/library/insertables/fasten-query.ts, leaving parse-fasten to parsing alone. configurations keeps the domain the frontend shares (models, canonicalization, combinations, input parser); its Onshape parsers moved to load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
auth/ had three files that all answered 'is this caller allowed' without a
line between them. Split by role instead:
- session.ts the session cookie and its KV records
- onshape-oauth.ts the handshake only
- caller.ts resolving who is calling, with the KV memoization, plus
productionCaller (was services.ts)
- guards.ts both gates, which were one-per-file in sign-in.ts and
access-control.ts
- routes.ts the OAuth redirects and /access-data
/access-data reports access level and sign-in state, so it moves from
users to auth. What is left of users is the caller's stored preferences,
so it becomes settings/.
app.ts was a composition root plus the /init handler plus error handling.
/init is now features/entry (its test came along, as routes.test.ts
rather than app.test.ts), the error handler is lib/errors.ts, and the
caller binding is lib/context.ts, leaving app.ts to mount and nothing else.
AppServices is renamed Caller: it is the request's caller, not a service
registry. not-signed-in.test.ts is split into the modules it covers,
guards.test.ts and routes.test.ts.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Matches the feature that serves it and what it actually updates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
configurations.id is a 1:1 FK to insertables.id, so LibraryOut's configurationId was always just the insertable's own id: a boolean wearing an id costume. Every consumer either truthiness-tested it or passed it straight back as the id. InsertableOut now carries isConfigurable, and callers fetch by insertable id. The route param follows suit (/api/configuration/:insertableId), and the configuration query keys gain insertableConfigurationQueryMatchKey for the prefix match insert-menu was spelling inline. Adds tests pinning the behavior this rests on: the library response marks configurability but carries no parameters or records, which stay in D1 until an insertable actually needs them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
An element's part number, name and material were kept as the first entry of configurations.records — the probe of its default configuration. Every probed insertable therefore carried a configurations row, including ones with no parameters to configure, and "is it configurable?" had to test the parameter count rather than the row's existence. That part data is not a configuration of the element, it is the element, so it moves to insertables.part_data. configurations is left holding only configuration data: a row exists exactly when there are parameters, and isConfigurable is now just whether the row is there. ConfigurationRecord becomes PartData plus the configuration that produced it, and toRecords() recomposes the full list — the element's own data as the record an unset configuration falls back to, then one per indexed configuration — for search and the configuration endpoint. Both paths are now covered by tests that fail without it. The migration backfills part_data from records[0] (always the default probe; see toResult), strips it from records, and drops the rows left with no parameters. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
The library reloads from Onshape, so the column starts null and repopulates on the next load rather than being carried over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
…dions Icons: all 34 Tabler icons map to a Phosphor equivalent, each name checked against the installed package rather than guessed. A filled heart becomes weight="fill" and Tabler's title prop becomes Phosphor's alt, which renders the same <title> element. notifications.tsx imported ReactNode from @tabler/icons-react, which does not export it; it now comes from react. Home page: the library and search sections were two near-identical Accordion.Item blocks, with their open state read from a hand-built array and written back through a branch on whether a search was active. Both are now one list of sections, each carrying where its own open state lives, so the value and onChange plumbing stops branching. The accordion divider was styled onto the panel content, so a collapsed section had no line under it. It moves to the control, which is rendered either way; content keeps the closing line when open. Verified in the running app. Also deletes the beta-complete page, which nothing linked to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
useAccessData falls back to a signed-out placeholder while access-data is in flight, so the navbar rendered the sign-in button on every load and then removed it once the response said the caller was already signed in. ResolvedAccessData now reports isLoaded, and the button waits for it. Anything gated on signedIn being true was already safe — it renders nothing until the data arrives; only the signed-out branch could flash. The toast's action button sat in a wrap="nowrap" row with no flex-shrink of its own, so a long message shrank it and clipped the label — 31px of the 47px "Sign in" needed. It no longer shrinks, and the message wraps instead. Both verified in the running app: the button never appears across a delayed access-data response, and the label now measures 47/47. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Mantine's Notifications containerWidth defaults to 440px and never grows, so a message with an action button wrapped to two lines at every window size - measured identically at 800px and 1400px wide. Setting it to max-content sizes the toast to its content and lets Mantine clamp it to the viewport when there is genuinely no room. Measured across widths: 360 -> 328px/3 lines, 420 -> 388px/2 lines, 900 and 1400 -> 514px/1 line, none overflowing the viewport. flexShrink on the action button stays: it is what keeps the label whole in the narrow case where the row still has to wrap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
isLoaded was re-deriving what TanStack already reports. useAccessData now passes through isPending, which also fixes a failure mode the derived version had: on an errored access-data, data stays undefined, so the old flag would have hidden the sign-in button forever instead of offering it. useSaveSettings had the same placeholder problem in the other negative gate - a snapshot reading signed-out would silently persist to localStorage for a user who has a server-side row. It now resolves access data when the mutation runs, so it cannot act on a placeholder. Verified against a delayed access-data: no button while signed in, button once a signed-out answer lands, button offered after the request fails outright. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Editing now requires a session as well as the access level. Access level alone let a signed-out caller through wherever it is granted without one (a dev ACCESS_LEVEL_OVERRIDE), and answered 403 rather than 401 for everyone else. Both cases are now covered by tests. Routes: every body and query is a zod schema, and every entity id comes from a route-params helper. That adds a favoriteRoute/getFavoriteParam pair and moves three paths onto the existing convention - /favorite/:favoriteId, /default-configuration/favorite/:favoriteId, /configuration/insertable/:insertableId, /reload-group-thumbnail/group/ :groupId - with the frontend using the matching to*Path helpers. The previously-unchecked c.req.json<T>() casts on favorites, groups, insertables and settings were assertions, not validation. cache.ts: the cache-version check was a one-key zod schema guarding a "versioned: false" option nothing ever passed. Both are gone. ThumbnailWorkflow reads the microversion from the insertable it already resolves, rather than carrying one that may be stale by the time it runs. R2 thumbnail metadata is a typed ThumbnailMetadata rather than a bare Record, so an element's own thumbnail is tagged with the empty canonical configuration it represents instead of omitting the field. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
AlexKempen
force-pushed
the
claude/codebase-restructure-features-snex55
branch
from
August 21, 2026 13:41
cf0e650 to
287815b
Compare
dto.ts is contract.ts - the backend owns the contract, and DTO was an acronym that named nothing. JobStatus moves out of the library's contract into the load feature that produces it. libraryId is stored per user but was missing from Settings, so DEFAULT_LIBRARY_ID lived off in library-id.ts as a second default. It is now part of Settings and DEFAULT_SETTINGS, and SettingsUpdate is just Partial<Settings>. Favorite writes are all scoped to their owner. Reordering and setting a default configuration were keyed on the favorite id alone, so one user could rewrite another's. Scoping the WHERE rather than reading the row first means no extra query. PartData is PartMetadata: absent values are undefined rather than null, and ConfigurationRecord documents that it is the same fields plus the configuration that produced them. Each probe now records whether it resolved to an open composite, and toResult derives instability by comparing against the element's own probe, so no record carries a flag about a comparison it cannot see. The two configuration-count issues now name their limits: CONFIGURATION_LIMIT_EXCEEDED and MANUAL_INDEXING_REQUIRED, matching IndexingBand.EXCEEDED and .MANUAL. addBuildIssue always returns a new array, with a test for it. checkIndexedPartNumber takes probes, not probed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Every failure now answers with { kind, message }, where kind says what
the client should do with the message rather than leaving it to guess:
- handled the message is written for the user; show it
- notice the same, for an outcome that is not a failure
- internal the message is for the logs; the caller shows its own wording
The frontend switches on kind alone, so it handles an error it has never
heard of. Callers keep supplying the wording for their own context, which
is what internal errors fall back to.
That replaces three inconsistent paths: a hand-rolled body two group
routes returned, HTTPException messages that reached the user or did not
depending on the route, and zValidator's own 400 body - which never
reached the error handler at all, since it answers rather than throws.
Routes now use a validate() wrapper that throws instead.
Onshape failures get a mapping of their own, so what we are willing to
repeat is one function rather than a decision per call site: a 401/403
says to try signing in again, a rate limit keeps its retry-after, and
anything else stays generic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
An unknown library id now 404s instead of quietly redirecting to FRCDesignLib, which hid the bad url. That surfaced a latent crash: the root renders around the not-found, so it saw an unvalidated libraryId and handed MantineProvider an undefined primaryColor. isLibraryId moves next to the other library path helpers and both callers use it. Mantine has no relative-time helper (only @mantine/charts and @mantine/dates components, neither of which we depend on), but the platform does. formatRelativeTime is now Intl.RelativeTimeFormat, which produces the same strings, localized, and adds "yesterday". ApiErrorBody is a discriminated union keyed on kind, following the same BuildIssueOf pattern already used for build issues, so a kind carries exactly its own fields. retryAfterSeconds now belongs to a rate-limited error rather than sitting optional on every error. The notice kind is gone: nothing ever produced one, and it was not distinguishable from handled. showToast updates a live toast rather than hiding and re-showing it, so a loading toast becoming a success one stays in place. Standalone, getColorTheme defaulted the system scheme to light, ignoring the OS. It now takes the scheme explicitly and the root supplies Mantine's useColorScheme when Onshape has not put one on the url. IconColor and HeartIconColor are gone - the latter was a duplicate of IconColor.RED. Phosphor icons render currentColor, so passing them through Box lets Mantine colors work directly. PrimaryColor and HEADER_CONTROL_COLOR stay: one is the dynamic per-library primary, the other is deliberately a hex. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
IconSize mixed magnitudes with one context name (CONTROL). Usage says the split is real rather than accidental: TINY, SMALL and MEDIUM are general sizes for an icon in a line of content (55 of 64 uses, across buttons, menus, toasts and rows), while the three largest each have exactly one placement. So the three general ones keep magnitude names and the rest say where they go: CONTROL stays, LARGE becomes SECTION and HUGE becomes PAGE. The doc comments now describe where each is actually used rather than where it was first meant to be. No size keeps its name with a different value, so nothing silently re-points. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
utils.ts held five unrelated things. Each moves to where it is used: getQueryUpdater and patchQuery become lib/query-cache.ts, useIsHome joins the other library route helpers, and capitalize and handleBooleanChange move into their single callers. The re-export of query-params went to api-client alone, which now imports it directly, so nothing is left and the file is gone. The Fast Refresh warnings were real, and the cause is the opposite of what it looks like: each menu file exports only its opener function while defining components privately. React Refresh can only swap a module whose exports are all components, so an edit invalidated the module and propagated to every importer - editing a menu remounted its callers instead of preserving their state. The openers move to sibling modules and the component files export their components, which silences the warning. Verified by driving HMR in a browser: the invalidate messages are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H
Give the cancel/restore toast an id keyed on the insertable, so opening and cancelling the same one repeatedly refreshes a single toast instead of stacking up a column of them. showInfoToast now takes an options object rather than a bare id, since it also needs autoClose. Mount hono/logger on the app. Only the Worker routes run it (assets are served ahead of the Worker by run_worker_first), and console.log reaches Workers Logs in production because observability is already enabled. Add a one-time tip after inserting an element's default configuration from the insert menu, pointing out that a right-click would have done the same thing. The condition is the canonical configuration being empty rather than "unchanged since the menu opened": right-click quick insert on a card passes no configuration, so a search hit's configuration is not what the tip would have gotten the user.
InsertableCard rendered its menu without a configuration, so right-click quick insert on a search hit inserted the element defaults while the card's own thumbnail and the insert menu both showed the hit's configuration. Pass it through, as FavoriteCard already does for a favorite's default configuration. With that fixed, the quick-insert tip can use the condition it should have: the configuration is still the one the menu opened with, compared canonically so an untouched menu counts as unchanged whether or not it was opened with a configuration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breaks up shared/types.ts, shared/api-models.ts, backend/app.ts,
backend/auth.ts and frontend/queries.ts along their seams so the
following move to a feature layout is a pure relocation.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_014UDJxumrXdB2NqtfyfsH2H