Uh oh!
There was an error while loading. Please reload this page.
feat(dfm): map features to directions, and plan the ways up - #1
Merged
Conversation
Ports the directions and mapping work from the sandbox: 67 new files, 38 merged into files that had moved here, and four conflicts resolved by hand. A part can now be mapped by feature — click a face, see every feature that owns it grouped by way up — or by direction, holding an arrow and painting the faces it reaches. Readings are mapped rough, finish or both; a face is cut once per pass, and giving it up is recorded. Identical holes collapse into one row. What is not cut yet is a list of faces, biggest gap first, opening onto the candidates that would cut them. The ways up themselves can be generated five ways, previewed on the part before being accepted, reordered into run order, and locked. Rules judge only what is mapped, on the band scale, and a band opens onto the features in it with the rules that cost them. Four files needed resolving rather than merging. Three were the DFM rename colliding with the Toolpath mark going in beside the name — the sandbox side already says DFM, so it wins. The fourth was rule-editor.tsx's import block, where this repo had gained a MatchRow editor the sandbox never saw: the imports are unioned, and rule-editor.test.tsx passes, which is the check that it went right. Not green yet, and cannot be until @toolpath/viewer 0.4.0 publishes (toolpath/ui-packages#51). Five things wait on it, all the same cause: the zoomTo, onAdjacency and widened shownDirection props do not exist in 0.3.1, and two selection-colour tests assert the retuned direction palette that ships with it. Everything else — 795 of 797 unit tests — passes here today.
`tests/cube-fixture.ts` read its report and mesh from `packages/viewer/fixtures/` — a workspace-relative path out of the app, which resolved in the monorepo this came from and resolves to nothing here. `@toolpath/viewer` publishes `dist` only, so there is nothing to reach for either. Both files are copied into `tests/fixtures/` instead. This is the fixture that mounts real geometry, and every spec that begins with a click on the part runs through it, so it took the whole e2e suite down rather than one test. 114 e2e, 111 passing. The three that fail — drawing a reading on the part, deleting a made reading, and keeping a keyboard walk through an orbit — all pass in the sandbox against the viewer's source, and all three go through the chaining that needs `onAdjacency`. 0.3.1 has no such prop, so React drops it and nothing ever reports which faces touch. They clear with the bump.
A reading cut down to some of its faces lost its difficulty colour completely, so the part went grey exactly where a reading had been divided — often where the hardest work is, and always where somebody has been making decisions. Only one of the two washes had a face-by-face layer. A split reading drops out of the by-tag map on purpose, because the viewer expands a tag to the faces the Engine reported rather than the faces the plan gave it. Directions caught those readings in regionWash; difficulty had nothing to catch them with. regionWash now answers for difficulty too, in the band of the reading cutting each face, with cutRegionsByFeature to say which reading that is. The band is still the Engine's verdict on the reading as reported — nobody has asked it what a reading cut to four of its six faces would cost. When edited features go back for analysis, that is where their answer arrives.
interactions.md still called the Directions tab a placeholder that prints the axis of whatever is being read. That is exactly what the mapping work replaced, and it is the first section of the first document a reader opens — so the app's own docs described a version of it that has not existed since the port. It also linked the specification at ../../../docs/directions-plan.md, a path in a repo this app no longer lives in. The plan and the findings are both in docs/ here now, so it points at those. Section 6's note that the missing scope flag 'stops being safe in PR 7 of the directions plan' was a forward reference to work that has since been done. The warning it carries is still worth having, so it is kept and put in the present tense rather than deleted. The rest of the document has not been audited line by line and some of it will be stale in smaller ways. This fixes what is flatly false.
…press A reading held by a locked setup moved anyway. The lock was drawn by the panel, explained by the face editor and recorded in the plan — and read by nothing except the generators, so every manual gesture walked through it. Two routes reached settled work: pressing R, F or Both on the reading itself, and cut-once quietly taking its faces for a reading nobody named in the press. The second is the one that hurts, because the press looks like it worked. setPassFor refuses the whole press either way — refusing only the locked half would leave two setups cutting one face, breaking cut-once. The row carries it too: a settled reading wears a lock naming its setup and its R/F/Both go inert with the reason in the title, still legible so the passes it holds can still be read. settledSetup is pulled out of face-list, which worked this out inline, so the two views cannot drift about what settled means.
…st its own A face mapped and settled from one way up kept live R/F/Both on every other reading of that same face. Pressing them did nothing. The plan was never in danger — setPassFor refused correctly. The refusal and the affordance were answering different questions: disturbsLocked asked whether the press would move settled work and knows about cut-once, while the row asked whether *this* reading was settled and does not. Those diverge wherever a face is read from more than one direction, which is everywhere. That is the failure 7d0c907 set out to kill, one layer out. It closed the route through the plan and left the one through the eye, and its own message names it: the press looks like it worked. lockedClaims gathers what the locks hold once per panel — asked per row it is the list's own N+1 over every feature, three times a row. blockedBy returns the setup rather than a boolean, so the inert control can name the lock to open, and disturbsLocked now delegates to it: the two came apart because they were written twice, and a test asserts they agree press for press. Asked per press, not once per row. A setup settled holding a face's rough has not settled its finish, so shutting the whole row would be the lock claiming ground it never took. All four call sites in the panel pass it now; only the feature rows were ever wired. State decides the colour and the block decides only whether it presses, so a settled reading keeps its lit R/F/Both. Greying every refused button alike made the one reading that was settled look as empty as the four refused on its behalf, which is the thing somebody opened the row to find out. Eighteen tests across the three layers, because the bug lived in the seam between them — and the wiring test is the one that matters, that being the layer that was actually wrong.
migration.md was the sandbox's copy, written before the port and still describing it as work ahead. It is rewritten from this repository's point of view, because the work landed here: what arrived, what state it is in, and what is outstanding. Every load-bearing claim re-verified rather than carried over — the five commits, the completeness check, the test counts, both npm versions, the release run that failed on ad65b43, and the two pending changesets that make 0.4.0 a shared release with the view-cube fix. New: the local override mechanism, which tarball came from which commit and why they are deliberately two different commits; @toolpath/ui as a first-class second publish, which the sandbox copy discovered late because it resolves the kit from its own workspace; and a stage 4 that drops the overrides and bumps both packages as one commit. The parity checklist is ticked, in two passes. First by mapping each behaviour to the tests that actually run, so a tick names its evidence — which leaves the uncovered ones as a short list rather than an impression. Then by hand against a real uploaded part, which settled four of the six and turned one into a bug. Worth saying plainly: the hand pass is what found the lock, and no amount of reading the test list would have. Three rules that every draft of this document carried as never seen on real geometry have now been seen. staging.md was never ported — it aimed the work at toolpath/apps/part-viewer, which PR #50 deleted — so the two links to it are repointed here.
The app named a zinc step at every turn: 445 uses of 22 greys, each of them a decision about contrast written down as a decision about darkness. That reads fine until somebody wants the other theme, at which point every one of them is a place to add a `dark:` twin. So the greys are gone and thirteen roles stand in their place — ground, surface, raised, lift; edge and its two louder cousins; ink down to ink-faint. The mapping was read off what each class was doing rather than guessed: a script took every utility, assigned it a role, and reported anything it could not place. Nothing was unplaced. `@theme inline` is what makes one class enough. The generated utility resolves the variable at use instead of baking its value in, so the same `bg-ground` follows the cascade into `.dark` and no component names a colour twice. The light ramp is written out rather than borrowed from the kit's zinc scale. That scale is tuned for dark surfaces — its `zinc-100` is `#e1e1e1`, a real grey rather than a hint of one, and panels painted with it read as off-white boxes on a white page instead of as the page. A script in the head owns the class, because it has to run before the first paint; reading the choice after mount is a flash of the wrong theme on every load. React must not also render it: hydration wrote the attribute back, so a light session went dark again on every navigation. One writer, the one that runs first, and `suppressHydrationWarning` on the element where they disagree on purpose. The model window flips with everything else. An earlier pass kept it dark on the reasoning that the direction cycle and the difficulty ramp are tuned against a dark ground — but the faces carry their own shading and their edges are drawn, so a part does not need a dark ground to be a part, and a light shell around a dark rectangle looked like a page that had not finished loading.
Typing a number into this app is not styling, it is behaviour, and it was earned: a controlled box that re-renders the parsed value cannot hold what somebody is halfway through typing — `0.` parses to 0 and comes back as "0", taking the point with it, so `0.156` is unreachable because the box eats the keystroke that would have got there. All of that lived inside `rule-editor`, where nothing else could reach it. The next thing to ask a shop for a measurement would have written its own, and written the bug with it. No behaviour change. `COMPLETE` and `Caption` travel with it, being the two things it cannot work without.
Paul's weights, rule for rule. The set had six distinct numbers across seventeen rules and ten of them sat at 2, so below the top few nothing was distinguishable from anything else — a standard drill size cost exactly what a sharp corner did. Both preset sets follow, since SendCutSend overrides thresholds and never weights. "Part size against the machine" is gone from the rules. It was never a property of a feature: banding it once per feature said the same thing about every pocket on a part that does not fit. It is a shop's answer now, under *What part sizes do you take?*, beside the other two decisions about a plan that are not scales. That control did not exist. `machine` was optional on the rule set, no preset set it, and nothing anywhere could enter one — so `partOverMachine` returned null and the rule it fed had never once fired. It also had no test at all, which is how it shipped dead. Both ends now, and either alone. Unset means the shop has not said, which is not a limit of zero: with neither end given the metric stands down rather than passing everything. One number for two bounds — too big and too small are both "not a part we take", and a shop that wants them graded apart wants two rules rather than one metric reporting a direction it cannot band. An end is all-or-nothing. A machine is three numbers and the part's sides are matched to them largest against largest, so two of the three is not a smaller answer, it is no answer. The half-filled triple lives in the component, so the judge never sees one. Still outstanding, and worth saying plainly: `useRules` is called without a bounding box, so `partSides` is null and nothing can read these sizes yet.
A shop reads in one unit and buys tooling in the other, and the sum between them is exactly the kind somebody gets wrong once and trusts afterwards. 0.125 in is a stock cutter and 3.175 mm is the same cutter; somebody typing one wants to recognise the other without switching the page and losing their place. So a measurement carries both readings rather than one, and the datasheet shows the second quietly beside the first. Rows that do not convert do not pretend to — a ratio and a count of faces are the same number either way, and a second reading of them would be noise dressed as precision. In the rule editor it is read off what is being typed rather than off the stored value, so it keeps up mid-entry, and it says nothing while the box is empty or half-typed. Tested both ways round, so neither unit is the special case: 25.4 mm is 1.000 in exactly, which puts a wrong factor in the number rather than in the last decimal.
…y what to press The two ends of the decision were a pair of small buttons crowded against a running count, which read as more settings rather than as the end of the panel. They have a rule above them now and room to be what they are: Cancel quiet, because leaving is always available and never the thing somebody came to do, and the press that acts saying what it will do to how many — the count read off the button about to spend it rather than off a caption competing with it. The empty directions panel said "Nothing is held yet", which names the state and not the way out of it. It now says what has not happened and both roads from there: the generator above, or the panel on the right, by face or by direction. Three tests for the two offers nothing was watching. `Pick directions` shares a branch with `From the rules` and differs only in what starts ticked — the rules pre-tick what the geometry forces, this starts from none — so it was a one-line ternary with two meaningful arms and one of them ever exercised. The chooser's reorder arrows had none either, and run order is the whole point of that press.
One shelf carried two different questions. *What is the part coloured by* is about the report and is the first thing anybody reaches for, so it keeps the corner the eye starts in and wears glyphs now. Arrows, zoom, grid and section change how you are looking rather than what at, and sit under the model where the hand already is after an orbit. The unit and the size go bottom right. Rough and Finish moved under the modes rather than beside them: with glyphs the row ran off the end of the canvas and under the panel, and the control it clipped was the one saying which pass you were looking at. The bottom strip is two rows because the canvas is only as wide as the panels leave it — about 480px on a 1400px window — and on one row the groups took turns hiding each other. It passes pointer events through everywhere but the shelves, a full-width bar over the canvas having eaten every drag that ended low. Nothing said how big the part actually was. It fills the viewport whatever its size and the report describes features rather than stock, so the size is measured off the geometry, sorted largest first — how the part happened to be drawn is not a fact about it — and pressing it swaps the unit for the whole page. And a banana, for scale. It answers before you have finished asking, which is why the joke has outlived every attempt to replace it with a legend. Converted from the 6.3 MB OBJ in `toolpath_ui` to a 746 KB GLB, geometry intact: dropping the normals is most of that, and computing them at load gives the smooth shading a banana wants anyway. Not preloaded — it is off by default and most sessions never turn it on. It is scene furniture, so it carries the frame flag: without it the direction arrows are placed against a box containing a banana, and point at a part-and- banana that is not what they mean. Framing both is `frameBox` instead, because a comparison with one side off screen is no comparison. Three glyphs are redrawn from the Toolpath UI set rather than imported. They live in `toolpath_ui`, which is a different package to the `@toolpath/ui` this app depends on — the name exists three times across the repos, and an import resolving to the wrong one would be a puzzle rather than an error. `@types/three` because `banana.tsx` is the first app file to import three directly. The runtime dependency was already there; this is its other half.
None of this branch's checks could run. `pnpm install --frozen-lockfile` failed on a `@types/three` entry added to package.json without regenerating the lockfile, so CI stopped at its first step and never reached the two failing unit tests or the three type errors behind them. Those all had one cause: the app is written against a `@toolpath/viewer` newer than the published 0.3.1. `zoomTo` is not a `Viewer` prop there, `onAdjacency` appears nowhere in its runtime, a list-valued `shownDirection` matches no index and so draws no arrows at all, and `DIRECTION_COLORS` is still the old palette the colour-separation test was rewritten away from. The viewer is packed from the local monorepo into `vendor/` and installed as a tarball rather than linked: a link resolves its peers from that monorepo, which gives two copies of `@react-three/fiber` and a banana that throws "Hooks can only be used within the Canvas component". `@types/three` moves to 0.185.0 to match the copy the viewer was built against. Coverage, at the seams that had none: - the part route's failed, waiting, no-job, malformed and refused-stream states, none of which any test reached - `theme.ts`, including running THEME_SCRIPT and checking it agrees with applyTheme — its try/catch swallows every way it can be wrong - `number-box.tsx`, whose tests stayed behind in the rule editor when it was lifted out, along with its second caller in the plan limits - `setups.ts`'s partial-claim readers, the four-bug distinction between what the Engine reported and what the plan cuts - `banana.ts` and `part-size.ts` And what the tests themselves were doing: - fixed sleeps in on-the-part become waits on the size readout, two frames, and settled transitions; the spec drops from 1.3 min to 47 s - perf.test asserts convergence to a fixed point instead of a stopwatch, which measured the runner as much as the allocator - forbidOnly and CI retries, so a stray `.only` cannot green the suite and a rare flake reads as flaky rather than red - the connect-upload-analyze route table and its datasheet, copied by hand into two specs, move into part-fixture - jest-dom is loaded rather than merely depended on, and the export-function declarations this branch added become arrow consts per AGENTS.md
Thirty-three function declarations across the shared modules and three components, written as arrows to match the rest of the codebase. Nine of them were never exported, which is why measurements.ts and paint.ts had both forms sitting next to each other. Mechanical: no signature, no ordering, and no logic changed. Every use-before-declare site sits inside another function body, so nothing evaluates in a temporal dead zone.
The three route modules used a default function declaration. React Router only asks for a default export, so a named const with a separate export line says the same thing in the codebase's own syntax. Kept apart from the rest of the sweep because it is the one part with any runtime risk: an anonymous default arrow would break React Fast Refresh, and a named one does not. The end-to-end suite covers these three as the app's entry points.
Nothing enforced the arrow-function convention, which is how thirty-six declarations accumulated behind a rule written down in AGENTS.md. This is a dependency-free check that reads the convention off the start of a line, so the word inside a doc comment or a sentence does not trip it. Runs first in pnpm check because it takes forty milliseconds, and its own tests join pnpm test:setup.
Three shelves floated on the canvas carrying three sets of numbers — the controls at `p-1` around 24px buttons, the unit at `p-1` around a smaller one, and the size reading at `px-2 py-1` with no wrapper at all. Side by side they sat at three heights and read as three unrelated things stuck to the part rather than as one row of controls. There is one description of a shelf now, and everything on the canvas wears it. Back on one line, too. They were stacked because the canvas is only as wide as the two panels leave it and the row used to overlap; it wraps now instead, so they are side by side wherever there is room and stacked where there is not, and never one control hidden under another. The panels were painted `surface`, which is a hint of grey meant for a hover and reads as an off-white box on a white page. A panel is not a box on the page, it is the page — so they sit on `ground`, and `surface` goes back to meaning "lifted", which is what it is for. Checked by sweeping every element wider than 120px for a background that is neither transparent nor white: before, two; now, none.
in/mm sits with the theme at the far end of the general shelf — both are how
somebody wants to read the page rather than what is on it — and the size stays
in its corner. Pressing the size still changes the unit, which is the quickest
way to do it and the reason it was ever a button.
The size is a reading, not a control. A shelf around it made it the loudest
thing on the canvas: three numbers with a border, level with the tools and
asking to be pressed as hard as the section switch. It is a plain line now, and
smaller.
Which turned up why it looked big. `button, input { font: inherit }` sat outside
every cascade layer, and an unlayered rule beats a layered one whatever the
specificity — so every font utility Tailwind generated for a button lost to it,
silently, in this whole app. `text-3xs font-mono` on that reading rendered as
16px Open Sans. The reset belongs in `@layer base`, and with it there the
reading went from 199px wide to 123px on its own.
The shelf is centred on the canvas again, and pinned by a test. It had been
dragged off twice: once by a flexible spacer that shrank to nothing and let the
reading print over the tools, and once by hiding that spacer without saying what
should centre the shelf instead.
Where the canvas is under 30rem the reading takes the line beneath rather than
overlapping. A container query and not a media one, because what decides is the
width the two panels leave — which somebody can drag without touching the
window. Checked at five widths for both centring and overlap.Editing `app/styles.css` blanked `pnpm dev` — black or white depending on the theme — and kept doing it until the server was restarted. Three times today. `./styles.css?url` handed React Router a URL to put in a `<link>`, and Vite invalidates a changed file by appending its own cache-buster to the request. That produced `/app/styles.css?t=1787777507502&url`, which the dev server 404s. No stylesheet, so React never mounted and the document came back empty. A restart cleared it because a fresh module graph has no `?t=`. Imported for its side effect now, which leaves the injection to Vite. The font sheet stays in `links()`, being a real URL on another origin. Proved rather than assumed: with the server running, appending a line to `styles.css` and reloading used to return an empty body, and now returns the app. Twice, once for the edit and once for taking it back out. The production build is unaffected — the end-to-end colour sweep would fail outright if the stylesheet went missing, and it passes.
The sizes were a number the app wrote down and never looked at again. `useRules` has always taken a bounding box and nothing ever gave it one, so `partSides` was null and every rule that reads the part rather than a feature stood down — the entry worked, the metric was tested, and neither could reach the other. The report is no help: it describes features, not stock. The only source of the part's size is the geometry on screen, and the viewer already measures it for the corner reading. So it hands the sides up, the inspector holds them, and the rules engine is finally told how big the part is. The verdict shows where the limits are typed. A number somebody enters and cannot see the effect of is one they have to go and check somewhere else, which is where a shop stops trusting it — so the card says whether this part fits and by how much it misses, in the unit being read. Both the card and the metric ask `outsideSizes`, which is lifted out for the purpose. Two callers answering the same question separately is how they come apart, which this app has already paid for once in the lock. The reading in the corner goes up a size. At 8px it was unreadable — that size only looked reasonable while a cascade bug was rendering it at 16px.
`whatBit()` was module state read through a getter, and all three callers reached for it on the line above the run that fills it — so the rules panel showed the previous arrangement's counters, and zeroes on the first press. A split-pass plan was worse: two runs, each resetting the ledger the other had just written, so only the finishing run was ever reported. `byBestReading` now returns the plan and its ledger together, which makes the ordering unwriteable, and `planForChosen` adds both passes up. Its six trailing positionals become an options object at the same time: the split call read `(..., limits, false, false, [pass], partial)`, and those two bare booleans had twenty lines of reasoning at the declaration and nothing at all where the decision was made. The rest is what the review of this branch turned up, taken in the same pass because it touches the same files: - `reach.ts` takes the reachability primitives and the three helpers that `best-reading` and `generate` each held a byte-identical copy of. The two already imported from each other, so the duplication was not buying a boundary; this breaks the cycle in the only direction it can go. - `part-click.ts` is the precedence table for a click on the part — which of four modes claims it — with its own tests. `pickFromPart` is now one effect per answer rather than a hundred and sixty lines of interleaved `if` and `setState`, where the order could only be read off the effects. - `keyIntent` does the same for the window's keydown handler. - `row-nav.ts` owns the `data-row` / `data-keynav` / `data-holes` contract. Seven components wrote those attributes by hand and three places read them by hand, so a rename broke keyboard navigation silently. - `PartViewProvider` carries the eight read-only values every panel was given identically. MapFeaturesPanel drops from 42 props to 35, FaceList 27 to 21, SetupsPanel 22 to 14. The callbacks stay props deliberately: they close over the page's state, and lifting them without stabilising identity first trades a long prop list for stale closures. - Around 230 lines of exported code no production path could reach, and the tests that only guarded it. `directions.ts` loses eleven of its seventeen exports — a way of naming orientations the app did not take. `disturbsLocked` looked dead by the same measure and is not: `setPassFor` calls it, and a lock that stops refusing presses is the bug it exists for. Unit tests 922 to 936. The end-to-end suite is unchanged at 133 passing and one failure, `a made reading can be deleted from its datasheet`, which fails the same way on the commit before this one.
Prettier and pnpm disagree about how to write pnpm-lock.yaml: Prettier breaks each resolution across several lines, pnpm writes it compact. With the file formatted, any ordinary install rewrote the whole thing, so a one-line dependency change arrived as a two-thousand-line diff with the real change buried in it. The lockfile is generated, so pnpm's style is the correct one. Ignoring it covers both routes that touched it — `pnpm format` and the lint-staged pre-commit hook, which filters its file arguments through this same file.
Button declared its label wrapper inside its own body, so every render gave that component a new identity and React remounted the label <div> rather than updating it. A browser only synthesizes a click on the nearest common ancestor of the mousedown and mouseup targets, so a re-render landing mid-press replaced the node under the pointer and the click never arrived. It showed up here as a Delete that did nothing when pressed straight after a row's onBlur wrote state, which is a real thing a person does. The fix belongs upstream and is made there; this vendors that build so the app and its mapping coverage are honest in the meantime. Revert to the published `0.1.1` and drop the tarball once it ships — the version on this one reads 0.1.0 but its bytes are not npm's 0.1.0. The lockfile is large because it is now pnpm's own formatting rather than Prettier's. Only the three @toolpath/ui lines are a change of substance.
Braces on every if, Array<T> over T[], path aliases instead of reaching out of the folder with ../, and ReactNode imported by name. All four were written down and none was checked, so the code had drifted a long way from the doc: 460 single-line ifs, 149 array annotations, 228 relative imports against a single alias import, six React.ReactNode. Mechanical only. ESLint --fix did the braces and the array syntax, a codemod did the aliases. The server keeps its relative imports into app/shared: production runs tsx with no bundler to resolve an alias. rule-presets.ts keeps its runtime import of FeatureType. It is a string enum, not the SDK client, and Object.values needs its members.
AGENTS.md stated six style rules and one of them was checked. Research on agent adherence is blunt about what that buys: how a rules file is written does not measurably change compliance, and compliance decays as a session runs long. A rule that is not a gate is a hint. So the rules become gates. eslint.config.js carries only rules that encode something AGENTS.md asks for, nothing type-aware, so pnpm lint stays cheap enough to run before the build. The layering was never checked at all. boundaries/dependencies now fails on app/ importing server/ by either alias or relative path, on a runtime SDK import outside the server, and on app/shared reaching into another layer. That needs eslint-import-resolver-typescript: without it the resolver cannot follow an extensionless .tsx import or a tsconfig alias, every dependency reads as unknown, and the whole rule is a silent no-op. Verified by breaking each boundary on purpose and watching it fail. check-style.mjs keeps the function-declaration rule alone. No rule is enforced twice.
Every styling rule now names the command that proves it, or is marked as judgment. That distinction is the point: a checked rule is not a matter of taste, and an unchecked one is a preference someone has to carry, which is what agents drift off first. When a judgment rule starts being violated it wants a check, not another sentence here. The style-prop rule was wrong rather than unenforced. All 28 uses are a band colour, a direction colour or a computed width, which Tailwind cannot express. Says so now. Project Map gains apps/dfm/docs/, which it had never mentioned. That folder already maps every behaviour to the file that decides it and was invisible to anything reading this document. review-code separates what pnpm lint proves from what needs a reader, and gains blast radius: what a plausible next requirement in this area would cost, with the files that amplify that cost listed by size and import count. review-testing points at the testing rules docs/README.md already fixed, and adds the question of which behaviour nothing would fail on.
Merges origin/paul/directions-mapping, whose four commits add the sizes a
shop takes as a judged rule, into the local branch's styling and layering
work. Every conflict was the same collision: props moving into context on
one side while the other added a new one.
- root.tsx: took the incoming plain `import './styles.css'`, which fixes the
dev server 404ing an invalidated stylesheet, and kept the `shared/theme`
alias. The unused `appCss` link goes with the `?url` import.
- feature-viewer.tsx: kept the new `onPartSides`; dropped `showingPass` from
the props, since it now comes from `usePartView()`.
- part-inspector.tsx: kept the context-fed prop list and added
`onPartSides={setPartSides}`.
- plan-choices.tsx: kept the incoming `outsideSizes` and `formatLength`,
written against the `shared/*` alias the lint rule now enforces.
- metrics.ts: braced the single-line `if` in `outsideSizes`, which arrived
before the `curly` rule existed.
`pnpm check` and all 135 end-to-end tests pass.The styling table said `@toolpath/ui` over hand-authored HTML was judgment, and the section above it says a violated judgment rule wants a check rather than another sentence. Nobody had counted this one: 78 raw `<button>` across 16 components, while the kit exports both `Button` and `IconButton` and both take `aria-*` and `title` through. A lint rule would have been 78 errors on day one, and migrating the components is a refactor with its own risk against 135 e2e tests. So kit-usage.test.ts pins the count instead: it may fall, it may not rise. A second case fails once the count drops ten under the budget without the budget following it down, so a migration keeps its ground. Verified by adding a button and watching it fail. Two rules that already had a sensor were not written down. styles.test.ts came in with the merge and proves the @layer base ordering and light/dark role parity — neither is visible in a component. Both join the table, which now ends by naming the three sensors that carry it. tsconfig kept a `server/*` alias that the table had already dropped and that nothing imports. A resolvable alias no document names is the footgun the boundary rule exists to close. Stale references, in docs promoted to steering by 2218092: - docs/README.md named `packages/viewer`, which does not exist here. It is the vendored @toolpath/viewer tarball, with no source in this tree. - The five tp-ui pointers now say plainly that none of them are in this repository and a template user cannot reach them. - migration.md was verified at 7d0c907 and read as current at a84b1db. The 26 Aug table is kept as the prediction record; the note above it carries the real numbers, 940 unit in 61 files and 135 e2e. - review-code's blast-radius table was stale by 10 and 25 lines. It is a command now, because a typed line count decays every commit. setup-testing is the skill that writes tests and did not know the two placement rules review-testing enforces, so it could produce exactly the test the review flags. It leads with all four now. review-testing points at them instead of holding a third copy, and audits the table in both directions. pnpm check passes; 135 e2e pass. No product code changed.
The part page holds thirty-odd pieces of state and re-rendered all five panels on each of them. Hovering a face row sets `hoveredFace`, which feeds the viewer's paint layers and nothing else, and the mapping, setups, rules and summary panels re-rendered anyway — twice per row the pointer crossed. `memo` could not fix that on its own: every handler was an arrow function rebuilt each render, so a memoised panel found fifteen new identities and re-rendered regardless. `useStable` gives a bag of callbacks one identity for the life of the page while still forwarding to the latest render's closure, which is what makes the comparison able to succeed. Two more things were separately defeating it, and neither is visible on screen — the panel simply renders more than it needs to, correctly: - `useRules` returned a fresh object literal each render, which is one new identity and enough on its own. - `uncut` and `handedTags` were built inline in the mapping panel's props, a fresh array and a fresh Set every time. Both now have tests, because nothing else can catch them coming back. Rules and Directions are loaded on the press that opens them: the route was one 1.36 MB chunk downloaded before the first paint of a page that opens on Inspector. 45 KB raw, 10 KB gzipped. The viewer is deliberately not split — it is on screen beside every tab, so deferring it buys nothing and costs a spinner over the main content. Also: the rule-explainer assertion re-hovers with the assertion rather than once before it. The tooltip opens 700ms after the pointer arrives, and anything moving the button inside that window cancels the open with nothing left to re-enter it — the wait then ran out against a button sitting right there. And a prop comment describing a `?axes` experiment that does not exist, which had attached itself to the prop below it.
All three are pre-existing on main — the branch's server diff is brace reformatting and nothing else — so none is a regression this branch introduced, and none was in scope for the four it did fix. Two are in server/, which is the API-key boundary, and are better done in a change that is only about the server than buried in a 40k-line client branch where a server edit is invisible. Each carries the evidence, the impact, and the fix, so none of it has to be re-derived. The last section records what the review checked and found clean — the report redaction, the upload path, the key handling — so that is not re-checked from scratch either.
Draw a reading, confirm it, then click one of its own faces: every reading of that face was listed except the one just drawn. It reached the plan and the left-hand panel, so it existed — it simply was not among the candidates for the faces it covers. `made` already says why this happens, a few hundred lines above the fault: readings drawn here are merged into one part rather than carried beside it, "because *every* list, the plan, coverage and the paint would otherwise each need to know about a second source — and the one that forgot would quietly leave a made reading out of the plan it is part of". The view context was handed the raw report and became that second source. Five components read `report.features` through it — the viewer, create, the face list, the datasheet, setups — and every one of them was given the merged part before the context existed. The viewer was the visible one: it answers "what owns this face" from the features it was given, so a face of a drawn reading came back owned by everything except it. The context carries the part now. One source again, which is what the comment asked for. Covered by the gesture rather than the plumbing: draw on two faces, leave Create, click one of them, and the drawn reading is in the list. It fails without the fix.
Two defects the review of paul/directions-mapping found and deliberately left for a server-only change, both pre-existing on main. The mesh retry overwrote its first Response without reading or cancelling the body, so undici held the socket until the object was collected. The retry runs whenever a 15-minute presigned artifact URL has expired -- somebody leaving a part open and coming back -- so this is the ordinary path, not a rare one. Cancel the discarded body before re-loading. getWholePartReport awaited each 50-feature datasheet batch before starting the next, so a 2,000-feature part was 40 sequential Engine round trips. It runs in the SSE handler on the succeeded event, so the browser sat on "Analyzing geometry..." for the whole walk after the analysis had already finished -- four seconds of pure latency at 100ms a trip. Four workers now drain a shared cursor over the batch list. Bounded on purpose: batching exists to stay inside a URL length limit, not to pace the Engine, but firing all 40 at once trades a latency problem for a load one. datasheetsByTag is written by every worker and stays safe because a feature tag belongs to exactly one batch. The code says so now that the loop shape no longer shows it. Both tests fail against the previous code: the mesh one gives the failing fetch a ReadableStream whose cancel sets a flag, and the datasheet one builds a 500-feature report and asserts peak in-flight requests stay above 1 and at most 4 with no datasheet lost.
2d1d784 vendored a build of @toolpath/ui to get the Button click fix before it was released, and named 0.1.1 as the version to revert to once it shipped. It has shipped. A tracked tarball cannot receive upstream fixes, pnpm audit cannot see inside it, and a diff to it is unreadable. 001985f already took @toolpath/viewer back to npm; this is the other half of that migration, so vendor/ goes with it, and the now-dead vendor/** ignore comes out of eslint.config.js. Unpacking both tarballs, the only differences are the version field and button.tsx: the published fix hoists the inner surface into a wrap(inner) helper returning an element, where the vendored build inlined it as a single div. Neither declares a component inside the render body, which is what caused the click to be swallowed, so both carry the fix. The dependency trees are identical. pnpm check and all 135 Playwright tests pass on the published build, tests/mapping.spec.ts included -- which is the coverage the tarball was vendored to keep honest.
f7eff96 recorded three reviewed defects left unfixed and why. All three are now fixed, so the page describes a state the repo is no longer in. Git history is the record of what they were; a standing document that says "not fixed" about fixed things is worse than no document. The one part with forward value was the list of things the review checked and found clean, which exists to stop the next review re-deriving them. That moves to AGENTS.md under the review guidelines, where a reviewer already reads. Re-verified rather than copied across: contracts.ts:28 still strips all three URL fields, and reading the SDK's own .d.ts, PartResponse and CreatePartResponse are still the only models in @toolpath/api 0.2.3 that declare a URL at all. That claim is now pinned to the SDK version, since a version bump is what can invalidate it. banana.glb is 763,952 bytes, which is 746 KiB and not the 763 KB the backlog reported from the byte count -- the figure in banana.tsx:149 was right. Recover the original with git show f7eff96:apps/dfm/docs/review-backlog.md.
…h-template into paul/directions-mapping
Two halves of the same wrong idea about how that list is used. Entering it turned every arrow on. The reasoning written down was that its filter is a click on an arrow and a mode whose only gesture is invisible is one nobody starts — but the gesture is a click on the **part**: find the gap, look at what could cut it. A way up is a narrowing somebody reaches for afterwards, if at all, and ten arrows over a list of faces bury the thing the list is about. They are left exactly as they were found now, on the way in and the way out. And a click on the part did not arrive in the list. Pressing a row already picks the face on the part — the comment there calls them "one gesture, two effects that belong together" — while the way back did not exist, so clicking the gap you were looking for left the list where it was and you scrolled to find your own place in it. A picked face now opens its row and brings it into view, while the uncut list has the panel and nowhere else, because everywhere else a face click means something entirely different. Both covered by the gesture, and both tests fail with their fix taken back out.
The fix pulled in on this branch repointed the part view's context at the part, which changed two panels rather than one. Its own test pins the face lists; nothing covered Create. Reverting the fix and running the suite said so exactly: one test failed, and `drawing over a face something already cuts says so before it is drawn` and `a reading already covering these faces says which way up it is cut from` both still passed. They draw over readings the Engine reported, so they never reach the list the fix changed. Create reads that list six times — the already-cut warning, `coveringAll`, the type guesses, the perimeter, the runs and the grown run. Drawing the same two faces twice is the shortest gesture that tells them apart: without the fix the second draft is told `This is new`, with the first reading sitting on the part covering exactly those faces. It fails without the fix.
`PartView` carried the whole report *and* a `features` beside it — two lists
that were equal only by convention, with a comment asking panels to read the
right one. The convention had already failed once: handed the raw report, five
panels read `report.features`, stopped seeing readings drawn on the part, and
clicking a face of a reading you had just drawn listed every reading of that
face except the one you drew. Pointing the field at the part fixed that and
left both lists in place.
It is one object now, named `part`, and `part.features` cannot disagree with
itself. The name is the load-bearing half: `report.features` reads as the
readings the Engine reported, which is exactly the wrong idea and is why five
panels reached for it.
Narrowing the type instead does not work, and the comment on the field records
why so the next person does not spend the afternoon finding out. `Omit`-ing the
readings type-checks clean and fails at runtime: `feature-viewer` spreads the
whole object into the mesh viewer's report prop, and `face-list` hands it to
`facesOf` and `cutElsewhere`, all three answering "what owns this face" from
the features they find on it. The spread satisfies the structural types, so
`pnpm check` passes and the original bug returns in silence.
`setups-panel` had been rebuilding the merged object by hand as
`{ ...report, features }`, which is the duplication in its plainest form.
Also: holding a way up dropped the readings drawn on it. `holdDirection` looked
each candidate tag up in the report, where a made reading is not, so the
predicate answered "no" and scoped it out of the list it belongs at the top of
— the same fault as the one above, in a closure the context change does not
reach. The predicate is `reachableFrom` in `shared/report` now, beside
`tagsOfType`, which already takes readings rather than a report for the same
reason; lifting it is what makes it testable at all, since the bug otherwise
sits behind an arrow drawn in WebGL.…h-template into paul/directions-mapping One conflict, in `on-the-part.spec.ts`, and it is the shape a conflict takes when nothing actually disagrees: both sides appended a test at the end of the file. Both are kept, in the order they were written.
Two things the uncut-list change left behind. Create *borrows* the arrows, and `showUncut` is one of the doors out of Create: it clears the draft. Taking the borrow and the return out of that function was right for the toggle itself — the list is read by clicking the part, and ten arrows over a list of faces bury it — but it also removed the only thing giving Create's loan back on that path. `arrowsBefore` stayed set and the part kept a full set of arrows nothing on screen explained, which is the exact condition that ref exists to prevent. The return moves to where the draft is cleared, so it belongs to the door rather than to the mode being entered. `leaving Create without choosing a way up takes its arrows with it` covers the By feature door and stayed green throughout; the new test covers this one. And the scroll went looking through the whole document for `[data-row="7"]`. The uncut list names a row by its region index, and the editor's face list names its rows the same way — two lists in one column, drawn at once, over the same numbers. A picked face with no row in the uncut list, because it is already cut or because the held way up filters it out, walked past this list and scrolled the other one. Scoped to `unmapped` now, and through `KEYNAV`/`ROW` rather than the attribute spelled out, which is what `row-nav` is for and what the identical effect in `face-list` already does.
The CSRF guard and secureHeaders could both be deleted with the suite still green: every mutating test sent Sec-Fetch-Site: same-origin, and nothing asserted a header. Pin a cross-site DELETE and a forged form post at 403, the four hardening headers, and no-store on /api. toPublicInspectionReport strips three named URL fields, which is a denylist correct only about the SDK it was written against. A fourth URL upstream would compile, pass contracts.test.ts, and reach the browser. redaction.test.ts builds its fixture from the installed PartResponse declaration and pins the URL surface of every generated model, so Region or PartFeature growing one is caught too. Also cover the branches nothing reached: a succeeded job with no report, an unreadable job event, an empty stream, a stream that ends mid- analysis, and both mesh_unavailable paths. The SSE failure test asserted only "status":"failed" and would have passed on a handler forwarding the Engine's own words; it now asserts the sentence and the server-side log. reported-regions matched its allowlist by file name, so any file under app/ sharing a basename was exempt regardless of layer. Match by path, and drop merge.ts, which interactions.md records as removed.
The ratchet listed app/components non-recursively, so a raw <button> in a route module or a folder added later was ground it never held. Walk app/ instead. The total is unchanged at 78 — every raw control still lives in app/components — so the budget stands. Prettier was enforced only by the pre-commit hook, which does not run on a rebase, an amend with --no-verify, or a web edit. Nothing in CI proved the rule. The new step caught its own first regression immediately. The captured-JSON rule was written absolutely and then contradicted by the next bullet, which mandates cube-fixture.ts. Say what is actually meant — no captured part reports — and name the vendored viewer cube as the one reasoned exception, in both places the rule is written.
The offer reads an analysis the Engine does not publish. Its SDK carries parts, features, jobs and keys, and nothing that returns a plan, so the button asked a question with nowhere to send it. Greyed in place rather than removed. The row is a set of alternatives, and a set that quietly changes size teaches people to count it again every time they open the panel — so it stays where it was, disabled, wearing a SOON flag and saying why in its tooltip. The empty state lists the same offers as advice, so it says it there too; without that it still sends people to a button that will not answer. The flag lives on GENERATORS, where the panel already reads everything else about an offer. Re-enabling it is one line, and `generate.test.ts` pins that nothing else carries the flag, so nothing else can go dark by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
pclauss123 added a commit
that referenced
this pull request
Sep 2, 2026
The branch was cut at 25c4d78 and main has moved 46 commits since — the whole of paul/directions-mapping merged as PR #1. Resolved on one policy: main's apps/dfm wholesale, this branch everywhere else, and the shared files merged by hand. - apps/dfm is main's, entire. This branch carried the template's older copy; Justin's is the one that landed. - packages/** is this branch's. Main's conflicting "packages" files are the DFM app's own app/shared modules that this branch extracted — same content, pre-extraction imports — and they came back with apps/dfm anyway. - Two of Justin's server fixes were ported into @toolpath/part-server, which is where that code lives now: the datasheet batches fetch concurrently under a cap of four, and a discarded mesh response is cancelled before the retry. His tests for both merged in with the package's suite and now pass. - Both applications are inside the sensors. AGENTS.md said to add apps/dfm to LINTED and SEARCHED_DIRECTORIES the day this branch landed; it has, so the layer patterns are apps/* and the API-key boundary covers both servers. AGENTS.md records the DFM app's own two sensors — the kit-usage ratchet and the stylesheet invariants — which the catalog has no equivalent for. - apps/dfm takes @types/three 0.185.4, matching the catalog: two versions of the same types made Box3 from one incompatible with Box3 from the other. pnpm check, pnpm audit, and both e2e suites pass: 140 DFM, 17 catalog.
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 freeto 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.
Ports the Directions work from
toolpath-sandboxand completes it here: mapping features to machining directions, planning the ways up, the rules changes that go with it, and a semantic colour system for light and dark.Scope
186 files changed (98 added, 88 modified), +41,796 / −3,713.
app/,server/, config)apps/dfm/docs/)pnpm-lock.yamlAdded files: 54 in
app/shared/, 25 inapp/components/, 8 intests/, 4 indocs/, 2 inscripts/.What changed
Mapping and planning — features map to candidate machining directions; a setup plan assigns readings per pass. A region is cut once per pass, so roughing and finishing are separate claims and either may be unset. Plan logic is in
app/shared/setups.ts,plan-actions.ts,plan-summary.ts, andgenerate.ts.Selection and interaction — click arbitration, multi-face holding, pick modes, and the Escape ladder are pure modules in
app/shared/(selection.ts,picks.ts,pick-mode.ts,part-click.ts,escape.ts,keys.ts,list-keys.ts), each with a colocated test.Rules — weights retuned.
Part size against the machinewas removed as a feature rule and added as a shop setting (What part sizes do you take?) with both ends optional.useRulesaccepted a bounding box that nothing passed, so part-level rules never ran; the viewer now measures the mesh and supplies it.Viewer — controls split into what the part is coloured by (top left) and how it is being viewed (bottom centre). Part size displayed in either unit. Adds an optional 746 KiB GLB scale-reference model (
public/banana.glb), off by default and not preloaded.Styling — 445 hardcoded greys replaced with 13 semantic colour roles, each defined under both
:rootand.dark.Server — datasheet enrichment now runs 4 batches concurrently instead of serially (
engine.ts); the mesh retry cancels the discarded response body before re-fetching (routes/mesh.ts); API-key validation and 404 handling added.Checks
CI (
quality) passes:pnpm install --frozen-lockfile,format:check,check(check-style, lint, build, check-types, test),test:e2e, anddocker:build.pnpm audit: no known vulnerabilitiesNo coverage tooling is configured, so there is no coverage figure.
Dependencies
All from npm, no overrides:
@toolpath/api0.2.3,@toolpath/ui0.1.1,@toolpath/viewer0.4.0.Checks added by this branch
Rules that were previously conventions now fail a command:
eslint.config.jsapp/cannot importserver/;app/shared/imports only itself; runtime SDK use confined toserver/scripts/check-style.mjsconst name = () => {}overfunction name() {}app/styles.test.ts@layer base; every colour role defined in both themesapp/kit-usage.test.ts<button>count ratcheted at 78 — may fall, may not riseapp/shared/reported-regions.test.tsregionIdxsrather thancutRegionsapp/shared/redaction.test.tsformat:checkin CIPre-existing bugs fixed
button, input { font: inherit }sat outside every cascade layer. An unlayered rule beats a layered one regardless of specificity, so font utilities on buttons and inputs had no effect. Moved into@layer base.app/styles.cssblankedpnpm dev:./styles.css?urlplus Vite's cache-buster produced/app/styles.css?t=…&url, which the dev server 404s, leaving no stylesheet and React unmounted. Changed to a plain side-effect import.Docs
apps/dfm/docs/carries the written spec (README.md,interactions.md,highlighting.md), the parity record (directions-parity-plan.md,directions-parity-findings.md,directions-replay.md), andmigration.md, which holds the parity checklist and the three behaviours with no automated cover.