Point & Click GDT Datum #8909

max-mrgrsk · 2025-11-13T21:58:05Z

Closes: #8577

datum-short.mp4

To do

Style Parameters Added:

src/lang/modifyAst/gdt.ts - code mod
src/lib/operations.ts - feature tree edit flow
src/lib/commandBarConfigs/modelingCommandConfig.ts - review validation
src/machines/modelingMachine.ts - event and state
src/lib/toolbar.ts - icon and button
src/components/layout/areas/FeatureTreePane.tsx - datum name is a value getValueDetail()
src/components/CommandBar/CommandBarBasicInput.tsx - fixed 1Password interference
src/lang/modifyAst/gdt.spec.ts - unit tests
e2e/playwright/point-click.spec.ts - e2e tests
src/components/layout/areas/FeatureTreePane.spec.ts feature tree test for getValueDetail()

Review Validation

similar to flatness, thanks @pierremtb

Checks engine connection
Applies AST modification
Mock executes via mockExecAstAndReportErrors() to validate generated code
Catches invalid parameters/selections before they reach app state
User cannot submit if validation fails

Quote Stripping & Validation

The || '' fallback in operations.ts enforces validation:

Edit mode: Extracts "A" from KCL + strips to A + shows in UI
Extraction fails: Returns '' + validation catches it with clear error message
No silent defaults
Command bar default: Schema has defaultValue: 'A' so UI always shows usable value

Notes for separate PR

Refactor all optional style related parameters into separate function for better reusability and maintaining all the upcoming features in gdt
Enable context menu on hover
You can trick the validation within the argument and select 2 faces
Can we actually see mockExec, that happens in the review phase?
Default plane and position should be normal to the face

Introduces addDatumGdt to create a gdt::datum call for a selected face in the AST. Ensures only one face is annotated, updates the AST with tagged face, and appends the annotation at the end of the AST body.

Added unit tests for the addDatumGdt function to verify correct datum annotation on cap, wall, and chamfer faces. Tests ensure proper tagging and GDT annotation insertion for each face type.

Added tests to verify that addDatumGdt fails when multiple or no faces are selected, ensuring proper error handling for invalid face selection scenarios.

Adds a check to ensure the datum name is a single character in the addDatumGdt function. Returns an error if the validation fails to prevent invalid datum names.

Added tests to ensure addDatumGdt rejects multi-character and empty datum names, verifying that only single-character names are accepted and appropriate error messages are returned.

Introduces the 'GDT Datum' command to the modelingCommandConfig schema and command set. This command allows users to add datum geometric dimensioning & tolerancing annotations to selected faces, with configurable node, face selection, and name. Marked as experimental and requires review.

Introduces the prepareToEditGdtDatum function to handle editing of GDT Datum operations, extracting face and name arguments and providing default values. Updates stdLibMap to use the new callback for 'gdt::datum'.

Changed the 'gdt-datum' button to trigger the 'Find and select command' for 'GDT Datum' in the modeling group. Updated its status to 'experimental' and revised the description for clarity.

Introduces support for the 'GDT Datum' modeling command, including event type, handler, and state logic. This enables experimental application of datum GDTs in the modeling workflow.

Added validation logic. Refactored gdt::datum call construction to use module path and updated related code to remove unnecessary mock execution error skipping.

Added calls to enginelessExecutor in GDT datum annotation tests to validate runtime consistency after AST modification. This ensures that the modified ASTs execute correctly in the test context.

vercel · 2025-11-13T21:58:10Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
modeling-app	Ready	Preview	Comment	Nov 17, 2025 7:10pm

Introduces getUsedDatumNames and getNextAvailableDatumName functions to scan AST for used datum names and suggest the next available name. Integrates getNextAvailableDatumName into the command bar config for datum creation, ensuring default names do not conflict. Adds related tests for both AST utilities and command bar integration.

Relocated deduplicateFaceExprs, exprToKey, getUsedDatumNames, and getNextAvailableDatumName functions from earlier in the file to the end. No functional changes; improves code organization and readability.

Added a check to reject datum names containing double quotes in addDatumGdt. Updated tests to verify validation error is thrown for such names.

Added data attributes (data-1p-ignore, data-lpignore, data-form-type, data-bwignore) to the input element to prevent password managers and autofill extensions from interfering with command bar input.

Enhances the OperationItem component to display the datum name for 'gdt::datum' StdLibCall operations by extracting and formatting the 'name' argument from the code. This improves clarity when viewing GDT Datum operations in the feature tree.

The addDatumGdt function now supports optional parameters for framePosition, framePlane, fontPointSize, and fontScale. These allow more control over the feature control frame's display and annotation text, and variables for these arguments are inserted only when needed.

Introduces a new GDT Datum button to the toolbar fixture and adds comprehensive end-to-end tests for creating, editing, and deleting GDT Datum features via the command bar. The tests cover all argument and optional parameter flows, including UI interactions and code verification.

pierremtb

Works great in Vercel Preview, and thanks for the comprehensive demo and details ❤️

Just a few comments before I approve, not all of them are 100% needed to merge especially as an experimental command.

pierremtb · 2025-11-17T19:53:59Z

src/components/CommandBar/CommandBarBasicInput.tsx

+          data-1p-ignore
+          data-lpignore="true"
+          data-form-type="other"
+          data-bwignore


Oh wow you had your pw manager try to fill this up?

pierremtb · 2025-11-17T20:10:43Z

src/components/layout/areas/FeatureTreePane.spec.ts

+    it('should extract datum name from GDT datum operation', () => {
+      const mockCode = 'gdt::datum(face = extrude001, name = "A")'
+      // Calculate sourceRange positions dynamically
+      const nameStart = mockCode.indexOf('"A"')
+      const nameEnd = nameStart + 3 // Length of "A" including quotes
+      const nameSourceRange: [number, number, number] = [nameStart, nameEnd, 0]
+      const mockOperation = createGdtDatumOperation(nameSourceRange)
+
+      const valueDetail = getFeatureTreeValueDetail(mockOperation, mockCode)
+
+      expect(valueDetail?.display).toBe('A')
+      expect(valueDetail?.calculated).toEqual({ type: 'String', value: 'A' })
+    })


Should we add another test case for getFeatureTreeValueDetail about VariableDeclaration types?

pierremtb · 2025-11-17T20:12:34Z

src/lang/modifyAst/gdt.spec.ts

+  |> close()
+extrude001 = extrude(profile001, length = 10)`
+      const ast = assertParse(code, instanceInThisFile)
+      const { getUsedDatumNames } = await import('@src/lang/modifyAst/gdt')


Let's move that import to the top?

pierremtb · 2025-11-17T20:14:06Z

src/lang/modifyAst/gdt.ts

+  nodeToEdit?: PathToNode
+}): Error | { modifiedAst: Node<Program>; pathToNode: PathToNode } {
+  // Clone the AST to avoid mutating the original
+  let modifiedAst = structuredClone(ast)


Let's also create a copied nodeToEdit here, so that mock exec doesn't mutate it.

const mNodeToEdit = structuredClone(nodeToEdit)

and then use it throughout. Check other codemods for ref!

pierremtb · 2025-11-17T20:14:40Z

src/lang/modifyAst/gdt.ts

+  const faceSelections = faces.graphSelections.filter((selection) =>
+    isFaceArtifact(selection.artifact)
+  )


Don't mind the extra step here, just wondering why this isn't a guarantee from the command bar?

pierremtb · 2025-11-17T20:16:30Z

src/lang/modifyAst/gdt.ts

+    if (typeof framePlane === 'string') {
+      // Named plane like 'XY', 'XZ', 'YZ'
+      framePlaneExpr = createLocalName(framePlane)
+    } else {
+      // Variable reference
+      framePlaneExpr = valueOrVariable(framePlane)
+    }


I know you mentioned it in the note, would likely be good to get a helper fn here

pierremtb · 2025-11-17T20:19:26Z

src/lang/modifyAst/gdt.ts

+  // Add optional labeled arguments if provided
+  if (framePositionExpr !== undefined) {
+    labeledArgs.push(createLabeledArg('framePosition', framePositionExpr))
+  }


I think I have a slight pref for the pattern at

modeling-app/src/lang/modifyAst/sweeps.ts

Lines 262 to 264 in 3a7f58d

const sectionalExpr = sectional

? [createLabeledArg('sectional', createLiteral(sectional))]

: []

and other codemods, of setting an empty array for that expr and then reconstructing it all in one array declaration but won't die on that hill

pierremtb · 2025-11-17T20:27:34Z

src/lib/commandBarConfigs/modelingCommandConfig.ts

+        defaultValue: (commandBarContext) => {
+          return getNextAvailableDatumName(kclManager.ast)
+        },


Suggested change

defaultValue: (commandBarContext) => {

return getNextAvailableDatumName(kclManager.ast)

},

defaultValue: (_) => getNextAvailableDatumName(kclManager.ast),

pierremtb · 2025-11-17T20:29:38Z

src/lib/operations.ts

+
+  // Extract name argument as a plain string (strip quotes if present)
+  const nameRaw = extractStringArgument(operation, 'name')
+  const name = nameRaw?.replace(/^["']|["']$/g, '') || ''


I think that regex should be part of a tested util fn?

pierremtb · 2025-11-17T20:31:13Z

src/machines/modelingMachine.ts

+      }: {
+        input: ModelingCommandSchema['GDT Datum'] | undefined
+      }) => {


I think we've been inconsistent with this due to branches in parallel, pretty sure @nadr0 introduced the dep injection here with #8511, check the other actors, maybe some need updating on main

max-mrgrsk added 11 commits November 13, 2025 17:14

Add function to annotate AST with datum GD&T

e087934

Introduces addDatumGdt to create a gdt::datum call for a selected face in the AST. Ensures only one face is annotated, updates the AST with tagged face, and appends the annotation at the end of the AST body.

Add tests for addDatumGdt on various face types

4bbe744

Added unit tests for the addDatumGdt function to verify correct datum annotation on cap, wall, and chamfer faces. Tests ensure proper tagging and GDT annotation insertion for each face type.

Add tests for datum annotation face selection errors

02cf17c

Added tests to verify that addDatumGdt fails when multiple or no faces are selected, ensuring proper error handling for invalid face selection scenarios.

Validate datum name length in addDatumGdt

41ffa9b

Adds a check to ensure the datum name is a single character in the addDatumGdt function. Returns an error if the validation fails to prevent invalid datum names.

Add tests for datum name validation in addDatumGdt

06d0293

Added tests to ensure addDatumGdt rejects multi-character and empty datum names, verifying that only single-character names are accepted and appropriate error messages are returned.

Add prepareToEdit callback for GDT Datum operation

1a57cb0

Introduces the prepareToEditGdtDatum function to handle editing of GDT Datum operations, extracting face and name arguments and providing default values. Updates stdLibMap to use the new callback for 'gdt::datum'.

Update GDT Datum toolbar button behavior

2929e0f

Changed the 'gdt-datum' button to trigger the 'Find and select command' for 'GDT Datum' in the modeling group. Updated its status to 'experimental' and revised the description for clarity.

Add GDT Datum command to modelingMachine

3850974

Introduces support for the 'GDT Datum' modeling command, including event type, handler, and state logic. This enables experimental application of datum GDTs in the modeling workflow.

Review validation implementation

f2a85db

Added validation logic. Refactored gdt::datum call construction to use module path and updated related code to remove unnecessary mock execution error skipping.

Add runtime consistency checks to GDT tests

2def4b4

Added calls to enginelessExecutor in GDT datum annotation tests to validate runtime consistency after AST modification. This ensures that the modified ASTs execute correctly in the test context.

max-mrgrsk linked an issue Nov 13, 2025 that may be closed by this pull request

Point & Click support for GD&T Datum (MBD) #8577

Open

max-mrgrsk self-assigned this Nov 13, 2025

max-mrgrsk added feature Feature Requests area/ast area/tests/e2e high-priority area/tests/unit area/point-and-click area/command-palette Anything to do with the command palette (or command bar) system labels Nov 13, 2025

max-mrgrsk added 2 commits November 14, 2025 16:29

Move utility functions to end of gdt.ts file

3102ca0

Relocated deduplicateFaceExprs, exprToKey, getUsedDatumNames, and getNextAvailableDatumName functions from earlier in the file to the end. No functional changes; improves code organization and readability.

vercel bot deployed to Preview November 14, 2025 15:50 View deployment

Validate datum names against double quotes

601561e

Added a check to reject datum names containing double quotes in addDatumGdt. Updated tests to verify validation error is thrown for such names.

vercel bot deployed to Preview November 14, 2025 16:07 View deployment

max-mrgrsk added 2 commits November 14, 2025 18:24

Add autofill prevention attributes to input

9b4eeb9

Added data attributes (data-1p-ignore, data-lpignore, data-form-type, data-bwignore) to the input element to prevent password managers and autofill extensions from interfering with command bar input.

Show datum name in the feature tree

0ccf69d

Enhances the OperationItem component to display the datum name for 'gdt::datum' StdLibCall operations by extracting and formatting the 'name' argument from the code. This improves clarity when viewing GDT Datum operations in the feature tree.

vercel bot deployed to Preview November 14, 2025 19:20 View deployment

vercel bot deployed to Preview November 17, 2025 13:52 View deployment

Merge branch 'main' into max/gdt-datum-ui

2b67cd2

vercel bot deployed to Preview November 17, 2025 14:35 View deployment

max-mrgrsk marked this pull request as ready for review November 17, 2025 15:46

max-mrgrsk requested a review from a team as a code owner November 17, 2025 15:46

Merge branch 'main' into max/gdt-datum-ui

b9ca345

vercel bot deployed to Preview November 17, 2025 19:10 View deployment

pierremtb reviewed Nov 17, 2025

View reviewed changes

	const sectionalExpr = sectional
	? [createLabeledArg('sectional', createLiteral(sectional))]
	: []

Point & Click GDT Datum #8909

Are you sure you want to change the base?

Point & Click GDT Datum #8909

Uh oh!

Conversation

max-mrgrsk commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To do

Style Parameters Added:

Review Validation

Quote Stripping & Validation

Notes for separate PR

Uh oh!

vercel bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pierremtb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

max-mrgrsk commented Nov 13, 2025 •

edited

Loading

vercel bot commented Nov 13, 2025 •

edited

Loading