From fc80fdfea855f0395f4f0e88bf7e2575121019da Mon Sep 17 00:00:00 2001 From: Justin Gray Date: Fri, 11 Sep 2026 15:12:03 -0400 Subject: [PATCH 1/2] Fold a group's reach curves into one, not the last clicked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The curve the drawing, the sweep and the tool list were all judged against came off `reading` — the face clicked last — so a group of six was answered against whichever of the six that happened to be, and the verdict changed when nothing about the question had. One tool for all of them has to get past all of their material, so the curve is now the pointwise maximum of theirs. A reach curve is a staircase of heights by offset out from the cut, read up from the bottom of the feature — which is where the tool tip sits when it is in that feature — so every curve is already in the tool's own frame and the max is exact rather than an approximation. `groupCurve` in shared/group-geometry.ts is the fold: every input knot is a knot of the answer, heights come from `heightAt` so the drawing and the sweep cannot disagree about where a rise comes, and a knot whose height the run after it repeats is dropped. `askedCurve` beside it is the scope — `asked()`'s answer, not one feature of it. A group answered one tool each still reads the feature in front of it, since every feature there gets its own tool. --- AGENTS.md | 1 + apps/catalog/app/routes/part.tsx | 25 +++- .../catalog/app/shared/group-geometry.test.ts | 108 +++++++++++++++++- apps/catalog/app/shared/group-geometry.ts | 82 ++++++++++++- docs/FEATURE-LIST.md | 16 +++ 5 files changed, 224 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7912a49..77ada26 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -304,6 +304,7 @@ application unless that application says otherwise. | whether the rack shows them at all, and how many | `holdersToOffer`, same file | | the press that shows them, hidden to begin with | `app/components/no-collet-toggle.tsx` | | a group's worst case, and whose it is | `app/shared/group-geometry.ts` | +| the material a whole question has to clear | `askedCurve`, same file | | how far below the holder a stack has to stand | `belowHolder`, `app/shared/drawn-assembly.ts` | | which slots were filled against the rules | `overrides`, `app/shared/assembly-tree.ts` | | what a stack offers, and its button's words | `app/shared/assembly-actions.ts` | diff --git a/apps/catalog/app/routes/part.tsx b/apps/catalog/app/routes/part.tsx index 2287418..567209e 100644 --- a/apps/catalog/app/routes/part.tsx +++ b/apps/catalog/app/routes/part.tsx @@ -82,7 +82,7 @@ import { orderAssemblies, type ComponentSort, } from 'shared/order-list' -import { groupReadings, sharedHoleDiameter } from 'shared/group-geometry' +import { askedCurve, groupReadings, sharedHoleDiameter } from 'shared/group-geometry' import { groupOffer } from 'shared/group-offer' import { usedElsewhere, usesByGuid } from 'shared/component-usage' import { toolActionLabel, toolActions, type ToolAction } from 'shared/tool-actions' @@ -210,7 +210,6 @@ import { splitHolding, tapers, } from 'shared/holding' -import { sectionOf } from 'shared/section-of' import { belowHolder, type BelowHolder } from 'shared/drawn-assembly' import { drawable, holdable, holderOptions, policyOf, thresholdsFrom } from 'shared/holder-choice' import { closestMisses, closestPerForm, type Format } from 'shared/judge' @@ -1515,10 +1514,26 @@ const Inspecting = ({ report, jobId }: { report: PublicInspectionReport; jobId: return bore ?? holeDiameter }, [threadSpec, holeChoice.mode, holeDiameter]) - /** The reach curve the holders are swept over, read off the feature. */ + /** + * The reach curve the holders are swept over: **the whole question's**. + * + * It was read off `reading`, the face clicked last, so a group of six was + * drawn, swept and judged against whichever of the six that happened to be + * (Paul, 2026-09-11). One tool for all of them has to get past all of their + * material, so the curve is the union of theirs — `groupCurve` in + * `shared/group-geometry`, beside the fold that answers every other number a + * group is chosen against, and `askedCurve` is the scope it is folded over. + * + * The scope is what the page is being asked, and not one feature of it: the + * same `asked()` answer the tool list is judged against, so the verdict under + * the drawing and the column beside it are about the same thing. A group + * asked for one tool **each** is not one question — every feature gets its + * own tool — so that case reads the feature in front of it, as does a reading + * hovered while the list speaks for itself. + */ const curve = useMemo( - () => (reading ? (sectionOf(reading, report.features)?.curve ?? null) : null), - [reading, report.features], + () => askedCurve(askedNow.results, selectedFeatures, reading, report.features), + [askedNow.results, selectedFeatures, reading, report.features], ) const thresholds = useMemo(() => thresholdsFrom(), []) diff --git a/apps/catalog/app/shared/group-geometry.test.ts b/apps/catalog/app/shared/group-geometry.test.ts index 0774055..a2fd936 100644 --- a/apps/catalog/app/shared/group-geometry.test.ts +++ b/apps/catalog/app/shared/group-geometry.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' -import type { PartFeature } from '@toolpath/part-contracts' -import { groupReadings, sharedHoleDiameter } from './group-geometry' +import type { PartFeature, ReachCurve } from '@toolpath/part-contracts' +import { heightAt } from '@toolpath/catalog-data' +import { askedCurve, groupCurve, groupReadings, sharedHoleDiameter } from './group-geometry' /** * A feature as the kernel reports one, cut straight down. @@ -167,3 +168,106 @@ describe('the bore a group shares', () => { expect(sharedHoleDiameter([])).toBeNull() }) }) + +/** + * **One tool goes into all of them**, so the material it has to get past is + * every feature's at once. The curve was read off the face clicked last, which + * made the drawing and the verdict under it depend on the order a group was + * picked in (Paul, 2026-09-11). + */ +describe('the material a group has to clear', () => { + const walled = (tag: string, curve: ReachCurve): PartFeature => + feature(tag, 'Pocket', { + zMin: -10, + facts: { kind: 'Pocket', cd: { ignore: { min: 6 } } }, + reachCurve: curve, + }) + + /** What every reader of a curve agrees it means, asked across a span of offsets. */ + const sampled = (curve: ReachCurve | null): Array | null => + curve === null + ? null + : [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 20].map((offset) => heightAt(curve, offset)) + + it('takes the taller of the two at every offset, not the one clicked last', () => { + const shallow: ReachCurve = { horizontalOffset: [2, 8], verticalOffset: [4, 6] } + const deep: ReachCurve = { horizontalOffset: [5, 10], verticalOffset: [12, 30] } + const union = groupCurve([walled('shallow', shallow), walled('deep', deep)]) + + const tallest = sampled(shallow)?.map((height, index) => + Math.max(height, sampled(deep)?.[index] ?? 0), + ) + expect(sampled(union)).toEqual(tallest) + // Every input knot is considered, and the two the run after them repeats + // come back out: the deep wall is over the shallow one's head throughout, + // so the answer is a staircase of two rather than of four. + expect(union).toEqual({ horizontalOffset: [5, 10], verticalOffset: [12, 30] }) + }) + + it('answers the same whichever order the group was picked in', () => { + const a = walled('a', { horizontalOffset: [3, 9], verticalOffset: [7, 11] }) + const b = walled('b', { horizontalOffset: [4], verticalOffset: [25] }) + expect(groupCurve([a, b])).toEqual(groupCurve([b, a])) + }) + + it('drops a knot whose height the run after it repeats', () => { + const low = walled('low', { horizontalOffset: [1, 2, 3], verticalOffset: [5, 5, 5] }) + const high = walled('high', { horizontalOffset: [3], verticalOffset: [9] }) + + expect(groupCurve([low, high])).toEqual({ horizontalOffset: [3], verticalOffset: [9] }) + }) + + it("is one feature's own curve, unchanged, when it is the only one", () => { + const curve: ReachCurve = { horizontalOffset: [2, 8], verticalOffset: [4, 6] } + expect(groupCurve([walled('one', curve)])).toEqual(curve) + }) + + it('ignores a feature that states no curve rather than reading it as flat', () => { + const curve: ReachCurve = { horizontalOffset: [2, 8], verticalOffset: [4, 6] } + expect(groupCurve([walled('one', curve), hole('bare', 20, 8)])).toEqual(curve) + }) + + it('answers nothing where no feature states one', () => { + expect(groupCurve([hole('a', 20, 8), hole('b', 20, 8)])).toBeNull() + expect(groupCurve([])).toBeNull() + }) +}) + +/** + * Which features the curve is folded over: what the page is being asked, and + * not one feature of it. The drawing was reading the face clicked last while + * the tool list beside it was judged against the whole question. + */ +describe('the scope the material is folded over', () => { + const walled = (tag: string, height: number): PartFeature => + feature(tag, 'Pocket', { + zMin: -10, + facts: { kind: 'Pocket', cd: { ignore: { min: 6 } } }, + reachCurve: { horizontalOffset: [4], verticalOffset: [height] }, + }) + + const shallow = walled('shallow', 4) + const deep = walled('deep', 40) + + it('folds every feature asked about, whichever of them is being read', () => { + expect(askedCurve('all', [shallow, deep], shallow, [shallow, deep])).toEqual({ + horizontalOffset: [4], + verticalOffset: [40], + }) + }) + + it('reads the feature in front of it for a group answered one tool each', () => { + expect(askedCurve('each', [shallow, deep], shallow, [shallow, deep])).toEqual({ + horizontalOffset: [4], + verticalOffset: [4], + }) + }) + + it('reads the feature in front of it when nothing is being asked', () => { + expect(askedCurve('all', [], deep, [shallow, deep])).toEqual({ + horizontalOffset: [4], + verticalOffset: [40], + }) + expect(askedCurve('all', [], null, [shallow, deep])).toBeNull() + }) +}) diff --git a/apps/catalog/app/shared/group-geometry.ts b/apps/catalog/app/shared/group-geometry.ts index b07254d..9bdf007 100644 --- a/apps/catalog/app/shared/group-geometry.ts +++ b/apps/catalog/app/shared/group-geometry.ts @@ -1,6 +1,9 @@ -import type { PartFeature } from '@toolpath/part-contracts' +import type { PartFeature, ReachCurve } from '@toolpath/part-contracts' +import { heightAt } from '@toolpath/catalog-data' import { FIELDS, defaultsFor, readingsFor, type GroupBound, type Unit } from './feature-defaults' +import type { Results } from './feature-list' import { boreOf } from './hole-mode' +import { sectionOf } from './section-of' /** * What a group measures, when the group is asked as one question. @@ -168,3 +171,80 @@ export const sharedHoleDiameter = (features: ReadonlyArray): number } return held } + +/** + * The material a group's tool has to get past: **every feature's, at once**. + * + * A reach curve is a staircase of material heights by offset out from the cut, + * read from the bottom of the feature — which is where the tool's tip is when + * it is in that feature. So a group's curve is the pointwise maximum of its + * features': one tool goes into all of them, and a stack that clears the + * shallowest pocket and fouls the deepest wall does not clear the group. + * + * It was the *focused* feature's curve alone, which is the last face clicked. + * A group of a shallow open pocket and a deep slot therefore drew, swept and + * judged against whichever of the two happened to be clicked last, and the + * verdict under the drawing changed when nothing about the question had + * (Paul, 2026-09-11). + * + * **The rise comes at the start of each run**, which is the one thing every + * reader of a curve has to agree about — `heightAt` in `@toolpath/tool-support` + * is the reader, so the fold asks it rather than re-deriving the staircase. + * Every input knot is a knot of the answer: between two of them no input + * changes, so the maximum does not either. A knot whose height the next knot + * repeats is dropped, since it says nothing the run after it does not. + * + * `null` where no feature in the scope states one — the honest answer, and the + * one that draws a tool with no material beside it rather than a wall of zero + * height. + */ +export const groupCurve = ( + features: ReadonlyArray, + all: ReadonlyArray = features, +): ReachCurve | null => { + const curves = features.flatMap((feature) => { + const curve = sectionOf(feature, all)?.curve + return curve ? [curve] : [] + }) + const first = curves[0] + if (first === undefined) { + return null + } + if (curves.length === 1) { + return first + } + + const knots = [...new Set(curves.flatMap((curve) => curve.horizontalOffset))].sort( + (a, b) => a - b, + ) + const heights = knots.map((offset) => + curves.reduce((tallest, curve) => Math.max(tallest, heightAt(curve, offset)), 0), + ) + const kept = knots.flatMap((offset, index) => + index === knots.length - 1 || heights[index] !== heights[index + 1] ? [index] : [], + ) + return { + horizontalOffset: kept.map((index) => knots[index] as number), + verticalOffset: kept.map((index) => heights[index] as number), + } +} + +/** + * {@link groupCurve} over whatever the page is being asked. + * + * The scope is `asked()`'s answer and not one feature of it, so the wall under + * the drawing and the sweep that judged the list beside it are about the same + * thing. Two cases read the feature in front of them instead: + * + * - a group asked for one tool **each** is not one question — every feature + * gets its own tool, so there is nothing to fold; + * - nothing asked at all, where a reading hovered over the part is the whole + * question and the list below speaks for itself. + */ +export const askedCurve = ( + results: Results, + asked: ReadonlyArray, + reading: PartFeature | null, + all: ReadonlyArray, +): ReachCurve | null => + groupCurve(results === 'all' && asked.length > 0 ? asked : reading === null ? [] : [reading], all) diff --git a/docs/FEATURE-LIST.md b/docs/FEATURE-LIST.md index f97ac3a..2ea11f3 100644 --- a/docs/FEATURE-LIST.md +++ b/docs/FEATURE-LIST.md @@ -449,6 +449,21 @@ Opens the group editor, seeded with whatever is already clicked. was folded from, deduplicated, so a group of thirty-nine identical holes says its line once — and `featureTag` on the fold's answer still says it for anything that wants to. + - **The material around it folds the same way**, and is the one fold that is + not a number on the strip: a reach curve is a staircase of material heights + by offset out from the cut, read from the bottom of the feature — which is + where the tool's tip is when it is in that feature — so a group's curve is + the pointwise maximum of its features'. `groupCurve` in + `shared/group-geometry.ts`, over whatever `asked()` says is being asked — + `askedCurve` beside it is that scope — and `heightAt` in + `@toolpath/tool-support` is the reader it folds + through so the drawing and the sweep cannot disagree about where a rise + comes. It was the _focused_ feature's alone — the face clicked last — so a + group of a shallow pocket and a deep slot was drawn, swept and judged + against whichever of the two that happened to be, and the verdict under the + drawing changed when nothing about the question had (Paul, 2026-09-11). A + group asked for one tool **each** is exempt: every feature gets its own + tool, so there is nothing to fold. - **There is no confirm on the box** (Paul, 2026-09-09). _Create group and add tool_ and the **Cancel** beside it are gone: the group is created and put on the order list by the press under the stack that answers it, in one press. @@ -1009,6 +1024,7 @@ true of the work the worker does: | the list on screen | `app/components/feature-list-panel.tsx` | | building a group | `app/components/group-editor.tsx` | | a group's worst case, and whose it is | `app/shared/group-geometry.ts` | +| the material a whole question has to clear | `askedCurve`, same file | | the one bore a group shares | `sharedHoleDiameter`, same file | | what a click on the part means, group or not | `Interaction.collecting`, `part-interaction.ts` | | where identical holes are grouped, once | `{ type: 'group' }`, `part-interaction.ts` | From d987385aedcac770c858e9097273c6daa011eb82 Mon Sep 17 00:00:00 2001 From: Justin Gray Date: Fri, 11 Sep 2026 15:12:08 -0400 Subject: [PATCH 2/2] Stand the material wall off the cut, never on the centreline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A reach curve is measured out from the cut, so every radius the overlay draws is `cuttingRadius + offset` and the wall's inner face *is* the cutting radius. `(DC ?? 0) / 2` behind a `DC !== undefined` guard put that face at r = 0 for any tool stating no cutting diameter: the guard admits exactly the values `?? 0` then swallows. `geometry` is typed `Record`, but a catalog built from a vendor that published none carries `null` through the JSON, and null is not undefined — so the hatch was drawn from the centreline outward, straight through the tool it was meant to stand clear of. A positive diameter is now required. A tool that states none gets no wall at all rather than one drawn from a radius nobody stated, and the gaps go with it since there is no flank to measure from either. --- .../app/components/catalog-drawing.test.tsx | 65 +++++++++++++++++++ .../app/components/catalog-drawing.tsx | 36 ++++++++-- 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/apps/catalog/app/components/catalog-drawing.test.tsx b/apps/catalog/app/components/catalog-drawing.test.tsx index 73564eb..e594d5f 100644 --- a/apps/catalog/app/components/catalog-drawing.test.tsx +++ b/apps/catalog/app/components/catalog-drawing.test.tsx @@ -406,3 +406,68 @@ describe('the overlay this application draws', () => { ) }) }) + +/** + * **The wall stands beside the cut, never on the centreline.** + * + * A reach curve is measured out from the cut, so every radius the overlay draws + * is `cuttingRadius + offset` and the wall's inner face *is* the cutting + * radius. `(DC ?? 0) / 2` behind a `DC !== undefined` guard put that face at + * `r = 0` for any tool stating no cutting diameter — `null` and `0` are not + * `undefined` — so the hatch was drawn from the centreline outward, straight + * through the tool it was supposed to stand clear of (Paul, 2026-09-11). + * + * The two halves are the rule: a tool that states a diameter gets a wall at its + * flank, and one that does not gets no wall at all rather than one drawn from a + * radius nobody stated. + */ +describe('where the material is drawn', () => { + /** Every coordinate the material path visits, in the frame's own units. */ + const points = (container: Element): Array<{ readonly along: number; readonly across: number }> => + [ + ...(container.querySelector('[data-part="material"]')?.getAttribute('d') ?? '').matchAll( + /(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?)/g, + ), + ].map((found) => ({ along: Number(found[1]), across: Number(found[2]) })) + + it('stands the wall off the cut by the tool’s own radius', () => { + const container = drawn() + const across = points(container).map((point) => point.across) + + expect(across.length).toBeGreaterThan(0) + // The inner face is the cutting radius — DC 3 mm — and not the centreline. + expect(Math.min(...across)).toBeCloseTo(tool.geometry.DC / 2, 6) + expect(Math.min(...across)).toBeGreaterThan(0) + }) + + it('draws no wall for a tool that states no cutting diameter', () => { + const { DC: _dropped, ...rest } = tool.geometry + const undiametered: CatalogTool = { ...tool, geometry: rest } + expect( + drawn().querySelector( + '[data-part="material"]', + ), + ).toBeNull() + }) + + /** + * The case the old guard was written for and missed. The type says + * `Record`, but a catalog built from a vendor that published + * no cutting diameter carries `null` through the JSON, and `null !== + * undefined` — so this is the tool that drew its wall from the centreline. + */ + it('draws no wall for a tool whose diameter is null or zero', () => { + for (const stated of [null, 0]) { + StubResizeObserver.all = [] + const broken = { + ...tool, + geometry: { ...tool.geometry, DC: stated as unknown as number }, + } satisfies CatalogTool + expect( + drawn().querySelector( + '[data-part="material"]', + ), + ).toBeNull() + } + }) +}) diff --git a/apps/catalog/app/components/catalog-drawing.tsx b/apps/catalog/app/components/catalog-drawing.tsx index c1f481b..1ac09df 100644 --- a/apps/catalog/app/components/catalog-drawing.tsx +++ b/apps/catalog/app/components/catalog-drawing.tsx @@ -326,11 +326,35 @@ export const CatalogDrawing = ({ */ const outline = curve === null ? null : assemblyOutline(viewer) const verdict = curve !== null && assembly !== null ? clearance(assembly, curve, margins) : null - const cuttingRadius = (tool.geometry.DC ?? 0) / 2 + /** + * The flank the material stands beside, and `null` where the tool has none. + * + * **A reach curve is measured out from the cut**, so every r the overlay + * draws is `cuttingRadius + offset` and the wall's inner face *is* the + * cutting radius. `(DC ?? 0) / 2` put that face on the centreline for any + * tool stating no cutting diameter, and the guard beside it — + * `DC !== undefined` — let one through: a `DC` of `null` or `0` is not + * `undefined`, so the wall was drawn, from `r = 0`, straight through the + * tool it was supposed to stand clear of (Paul, 2026-09-11). The hatch + * covered the tool's whole `+r` flank and the drawing said the cutter was + * buried in the part. + * + * `typeof` rather than `!== undefined`, because the type says + * `Record` and a catalog built from a vendor that published + * no cutting diameter carries `null` at runtime — which is the case the old + * guard was written for and the one it missed. + * + * A tool with no cutting diameter has no flank, so there is nothing to + * measure a gap from either: `gaps` goes with it, `overlaid` turns off, and + * the sheet is the tool on its own. That is the honest picture — the + * alternative is a wall drawn from a radius nobody stated. + */ + const stated = tool.geometry.DC + const cuttingRadius = typeof stated === 'number' && stated > 0 ? stated / 2 : null const profile = - curve !== null && tool.geometry.DC !== undefined ? materialProfile(curve, cuttingRadius) : null + curve !== null && cuttingRadius !== null ? materialProfile(curve, cuttingRadius) : null const gaps = - curve !== null && outline !== null + curve !== null && outline !== null && cuttingRadius !== null ? tightestGaps(outline.segments, curve, cuttingRadius, margins) : null @@ -359,7 +383,11 @@ export const CatalogDrawing = ({ } className="size-full" > - {overlaid && profile !== null && gaps !== null && outline !== null ? ( + {overlaid && + profile !== null && + gaps !== null && + outline !== null && + cuttingRadius !== null ? ( <>