AI Slop Removal - #49
Merged
Merged
Conversation
The tip was capped at once ever, so the one time it fired was often a slow, deliberate insert. Time it from the menu opening instead: an unchanged insert within 1.5 seconds is one a right-click would have done, and is worth saying every time it happens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
`1"` tokenized to a bare `1`, which prefix-matches every 1.5", 10-32 and 16T in the library, so the sizes a user asked for were buried. The mark now stays on the number and ends its token, leaving `1"` an exact size while a bare `1` still prefixes them all. Phosphor deprecated the unsuffixed icon names in favor of `*Icon`. The local heart wrappers become FavoriteIcon/UnfavoriteIcon, which is what they meant anyway and leaves HeartIcon to Phosphor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Searching a size found the element but showed its default configuration: `1` prefix-matches a segment of every part number (0016 -> 16), which tied the real match in the name, and the tie went to the part number's first record. A term matched whole now outranks one matched as a prefix, and a size's unit is folded into the mark, so `1`, `1"` and `1 in` all name the 1" configuration rather than the .25" one listed first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
CardTitle took three thumbnail props only to forward them, so the rows that know which configuration to render now pass the CardThumbnail itself. The part name and number block becomes its own component rather than a nested ternary in the middle of the title. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Every list narrows its insertables one of two ways and then renders the same rows, but each caller resolved hits to insertables and tallied the filtering itself (filterResult2 beside filterResult). Both ways now return FilteredInsertables, and the favorites list splits into the search, the browse, and the cards they share. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Favoriting from the insert menu or a search result dropped the configuration in front of the user, so the new favorite came back as the element's default and had to be edited into the one just picked. The button and menu item now carry it, canonical, the way the favorite menu stores it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The app could only be signed in to: standalone there was no way to leave, short of clearing cookies. Sign-out drops the session and the access level resolved from it, then lands the caller back where they stood. Only offered outside Onshape, whose panel session is Onshape's to end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The tab stays, so the library is announced, but selecting it lands on a zero state instead of an empty library: its groups, favorites and search all render inside the route, so gating there covers them at once, and the loader stops fetching what nothing will show. The search row goes with them, having nothing left to search. Also says why the group-restore latch is there, which a reviewer had to ask: without it, leaving a group redirects straight back into it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Where a caller left off was client state, so the entry redirect could not see it and the app had to bounce itself into the group after loading, guarded by a module-level latch against redirecting straight back out. The group now sits in the user row beside the library it belongs to, and entry computes the whole landing url in one query: the join is also the check, so a deleted group, or one left behind by a library switch, lands in the library instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Six review notes, all pulling the same way — one local store the app reads from, instead of state spread across a second localStorage key, the url, and a hook returning a pair: - ui-state gains useGetUiState/useSetUiState, so a component reaches for the half it wants instead of indexing a tuple. - local-settings is gone: the caller's settings live in ui-state, which is now the source of truth for them. Saving still writes the row when they are signed in, which is what a browser running the app for the first time, and the Onshape launch, start from. - The theme comes off ui-state rather than the url, so a standalone visit with no parameters still renders in it. The entry redirect seeds the account's theme once; /app takes it and drops the parameter, and the retained set is now the launch parameters by name. - A sign-in leaves a flag in ui-state rather than a url marker, and returns to the entry point, which confirms it and resumes in one place. - AppSelect and its option helpers had one caller, so they collapse into a SettingSelect beside it: values in, capitalized labels out, no memos. - CHROME_BACKGROUND is jargon; it is the background of the bars framing the page, so FRAME_BACKGROUND. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The mark still ends its number, which is what the tests that remain cover; `1 in` still reaches the 1" configuration through the term scoring rather than through canonicalization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Trimming and case-folding were sprinkled across the record parser, the vendor lookup, the index and the frontend's display rule, and the placeholder rule ran only at render — so `N/A` reached the index and searching it returned noise, while "repeats the name" was written twice. `clean` and `equalsIgnoreCase` are now a shared leaf, and one `meaningfulPartNumber` says whether a number identifies anything. The frontend's displayPartNumber delegates to it, so what is shown and what is indexed cannot drift apart. The escaped-slash literals read as strings while they are being touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
One tokenizer ran over every field, so a part number was read as prose: 217-2600 became 217 and 2600, TTB-0016 lost its zeros, and the fraction in TTB-0016-5/32 was folded into a decimal. None of that identifies the part any more. Tokenizing is now field-aware. A part number is indexed as typed plus its segments, so it is found whole or by either half, zeros included. A name keeps the decimal canonicalization its sizes need, since the standards write the same measurement as 1/2" and 0.5". A query has no field, so it offers both readings — minus the pieces that would only flood: a bare size is not split into digits, and the lone letters left by splitting `n/a` are dropped. Commas no longer stick to the word before them, which the dimensions in a name are full of. The frontend scorer reads each field the same way, and the leading-zero underline hack goes with the zeros it worked around. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Some vendors genuinely sell parts without part numbers, so the warning fired as noise on them. Checking the format of a number that is there is the useful check, and that is a separate piece of work. Stored rows keep the issue until a reload recomputes them, where it renders as nothing at all rather than an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The rename refactor replaced the resolution chain with an if-chain and dropped its middle arm, so a record naming McMaster-Carr resolved to the insertable's tagging instead — which its test says it should not. Put the arm back, in the new shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Dropping every one-letter term took the first keystroke of every search with it, and answering as the caller types is most of what the search is for. Only a letter left behind by splitting a longer word is dropped now — the `n` and `a` of `n/a`, which was the case that wanted it. The same measurement reaches the library written both ways: one vendor writes .196 as .2 and the next writes .19. A name is stored and searched as both spellings, so either finds the part. Most numbers spell the same either way and cost nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The stray-letter rule was the wrong shape for what it was protecting against. Nothing carries `n/a` — ingest drops it — but typing it split into `n` and `a`, and a one-letter prefix answers with most of the library. The placeholder is now recognized where it is typed, by the same rule that drops it where it is stored, so it finds nothing and the rest of the query is read as usual. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The two branches were a 35-line ternary argument, which read as one expression when it is really the same push either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Seventeen icons were each wrapped in a Box to take a Mantine color, and the colors themselves were written at every control: c="red" here, color="yellow" there. AppIcon is that wrapper, and StatusColor names what each color means, so an error looks like an error everywhere and the theme's variables stay out of the markup. The build-status icons collapse into one mapping from severity, since their four branches differed only in the icon and the color. Elements become parts in the group's zero state, which is what a library holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
A rate limit reached the client as its own kind carrying the seconds to wait, which nothing on the client read: the loader is what waits, and all the caller can do is try again. It is a handled message now. 401 and 403 take its place as kinds, because those do differ in what the caller can do: a missing session is offered a sign-in in the toast itself, where before both said "try signing in again" and left them to find the button. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Names and comments first: the private writer was `setUiState` beside the public `useSetUiState`, the storage key was written twice, and both accessors called themselves asynchronous while being neither. The parse was the real problem. State stored by an older build, or naming a library that no longer exists, threw inside getUiState — which routes call before rendering, so the app would fail to start with nothing but clearing site data to recover. It falls back to the defaults now: losing a preference beats losing the app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
- The frontend's part-number module re-exported one backend function under a second name; its two callers now import the rule itself. - parseUrl parses an Onshape document url, so parseOnshapeUrl. - The feedback form url sat in url.tsx; it belongs to the setting that opens it. - The default access level is granted as well as viewed, so a dev override is not clamped back to user while access data is pending. - RequireAccessLevel asked "is it admin, or is it editor" in two branches, which is isWithinAccessLevel — and answers a required USER level correctly, which the branches did not. - useIsJobRunning is the read several places wanted, leaving the refresh on finish to its single owner. - SeverityCounts is an interface rather than an inline shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Canonicalizing a quantity rounded the parsed base value to seven decimals of its own choosing and assembled the unit string by hand, beside a module that already knows what a value is measured in and how close two of them have to be to count as one. formatBaseValue is that spelling: base units, to the decimals the tolerance distinguishes, so two values the parser reads as equal cannot key two renders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The build status mapped a configuration row into a shape that differed from it only by dropping the id, so the id joins the contract and the row goes through as it is. Its private cache gets the reason written down: the body is the same for every editor, but a shared cache would hand it to whoever asked for the url next. isSignedIn reads as the one question it answers, and clean stops saying "blank is nothing" twice. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
`c`, `i` and `warm` become `configuration`, `insertableId` and `renderThumbnail`, which is what the flag asks for: start the render this url is missing. `v` stays, being the cache version every immutable url carries, and the one stored urls already hold. The stand-in served while a configuration renders was cached for a minute, which is a minute of anyone asking that url getting the old answer after the real render landed. It is a body the url does not promise, so nothing stores it now: the next request is free to see past it, and polling for a thumbnail we want costs an R2 read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Reaching for the middleware meant a context flag telling it to undo itself, for the one route whose answers genuinely differ: stored bytes the url pins, a stand-in it does not, and a miss that must stay retryable. setCache says that per response, and the middleware goes back to being for routes whose every answer is alike. The query param is canonicalConfiguration, since that is what it is: a url that took an ordinary configuration would key a different render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Every c.json in the error handler cast its status through never, because ApiError carried a bare number and c.json takes the statuses a body is allowed with. Carrying Hono's ContentfulStatusCode instead drops all four casts and makes the constraint real: a 204 as an error status is now a type error rather than a response the client cannot read. The import is type-only, so the leaf the frontend imports stays free of Hono in the bundle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
"Caller" was already the word for the person making a request — it is in sixty comments — so a type of that name read as modelling them, when it is really the answers a route can ask for about them. CallerFactory compounded it with the Java-ism. RequestAuth is those answers, AuthResolver is how one request's are resolved, bindAuth puts them behind c.var, and productionAuth answers for real. caller.ts becomes request-auth.ts; the prose keeps saying caller, which is what it always meant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
A stored record keys on the canonical spelling of its selection, not on the selection as written, so call the field canonicalConfiguration everywhere it is stored or read — including in the D1 records column and the R2 search index. The parse side still holds the selection as sent to Onshape, which only becomes canonical in toResult, so it gets its own ProbedRecord. Stored records only carry the new field after a library reload. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Three things were called some variant of "configuration", and two of them shared a name under different types: the selection a user made, that selection normalized into a map, and the text form of that map addressing a render. There is no normalized map any more. canonicalizeConfiguration returns the text directly, so a canonicalConfiguration is always a string and a configuration is always ParameterValues — and the few places that want a selection back out of the text decode it there. Records, the search index and a favorite's stored selection all hold the text, which is also what the thumbnail url carries and what the R2 key hashes. Also collapses the two identical encoders into encodeConfiguration, and replaces the hand-rolled cyrb53 with crypto.subtle: Workers implements WebCrypto, so a truncated SHA-256 needs no dependency. That makes the key async, which every call site already was. Stored records only carry the new shape after a library reload. Favorites are converted in place by 0008, which json_each walks in parameter order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Storing only the canonical form lost data: canonicalizing drops every value that equals a parameter's default and every hidden one, so a favorite could not carry a string parameter the user typed. The row keeps the selection as made again, and 0008 is dropped rather than reversed — it never ran anywhere. The canonical form the thumbnail needs is computed in getFavorites instead, against the insertable's current parameters. Storing it alongside would go stale the moment a reload changed what a parameter defaults to, and computing it per card would be a parameters query per row. Also unnests the card's part name and number, which rendered a div of p's inside the p the title is, and so tripped a hydration warning. It is a line under the title, so it belongs beside it in the stack. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
A configuration only ever had to name one path segment, and url-encoding it does that — the way Onshape spells one — so the hash bought nothing but a digest to explain. Encoding keeps `/` and `;` out of the path, and a key stays readable, which a truncated digest never was. Drops toConfigurationKey and DEFAULT_CONFIGURATION_KEY, and with them the only reason thumbnailKey's callers were async. Also notes on canonicalizing that it is lossy, which is why a stored selection is never only its canonical form. Thumbnails stored under the old hashed keys are orphaned: nothing reads them, and the configurations that want them re-render on demand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Every comment over two lines is cut to two without losing its reason, the padded JSDoc blocks collapse to one line, and the comments that narrated the line under them or restated a component's own name are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
Inline style was re-stating what Mantine already exposes: flex, miw, maw and display are style props on every component, Center is a grid that places its one child, and Box takes miw where a bare div needed style to. What is left is genuinely custom, so it is named once rather than written out at each use: NO_SHRINK for holding an icon beside text that can outgrow its row, and maskedImage for painting an asset in the current text color. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
A component's props were declared three ways — an interface, an object literal at the signature, a destructure at the signature — so a reader had to find out which before reading the props. They are all interfaces now. Defaults were split the same way: destructure the rest of the props, then a `props.x ?? fallback` line below for the optional ones, which reads as though it were doing something more than defaulting. They land in the destructure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
One file held 34 declarations and every layer of the build card at once, while every other feature here is already several files. It becomes five, each one thing the card is made of: the badges and hover card that open it, the issue severities it summarizes, the admin toggles, the parsed metadata, and the two row primitives all of them share. The seams are one-way — sections is a leaf, issues and parsed sit above it, admin above those, and the card composes them — so the imports say which layer a component belongs to. Every component here took its props as a literal type at the signature; they are interfaces now, like the rest of the frontend. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
The big components were doing three jobs at once: holding state, wiring mutations, and drawing. What was state or wiring is now a named hook — useInsertSelection, useSetDefaultConfigurationMutation, useHomeSections, useDefaultConfiguration, useReportSelection — and what was a self-contained block of markup is a component: InsertMenuFooter, SectionAccordion. useMenuTitle replaces the same updateModal effect written out in two menus. It takes an undefined name to mean not yet known, which is what the effects it replaces guarded for: the opener has already set a real title, so writing an empty one over it would blank the header until the query lands. The last components taking an object type literal have props interfaces too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL
AlexKempen
changed the title
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.
Refactors a lot of the logic and code to reduce the amount of AI slop in the codebase.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_018tgfuCHjVFvejjMe7WmEFL