Skip to content

Parse KCL measurement units consistently - #1844

Open
JordanNoone wants to merge 5 commits into
mainfrom
codex/volume-units-1744
Open

Parse KCL measurement units consistently#1844
JordanNoone wants to merge 5 commits into
mainfrom
codex/volume-units-1744

Conversation

@JordanNoone

@JordanNoone JordanNoone commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

KCL measurement arguments parse directly into the modeling-command unit enums for volume, mass, density, surface area, center of mass, bounding boxes, and combined analysis. Enable the crate's clap feature and require kittycad-modeling-cmds >= 0.2.230, which includes volume's ValueEnum support and preserves existing unit abbreviations, defaults, and density aliases. The lockfile retains the published 0.2.231 already used by current main; the temporary Git patch and volume conversion helper are gone.

Regression coverage exercises all 14 measurement-unit flags: 71 accepted flag/value combinations and 14 invalid-value diagnostics with the exact available choices. It also checks file/KCL volume parity, analysis defaults, and all six density spellings across four flags.

Validation:

  • cargo test cmd_kcl::tests:: --locked --offline: 7 passed, 0 failed.
  • cargo clippy --all-features --all-targets --locked --offline -- -D warnings: passed.
  • cargo fmt --all -- --check and git diff --check: passed.
  • Updated the Nix vendor hash from checksummed crate tarballs and the exact Git dependency tree; verified the calculation by reproducing current main's existing hash. Full Nix builds run in CI.

The local tests exercise command parsing and settings without live Engine/API requests.

Fixes #1744

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.62%. Comparing base (6733975) to head (3db37ab).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1844      +/-   ##
==========================================
+ Coverage   66.18%   66.62%   +0.44%     
==========================================
  Files          43       43              
  Lines        8451     8563     +112     
==========================================
+ Hits         5593     5705     +112     
  Misses       2858     2858              
Flag Coverage Δ
unittests 66.62% <100.00%> (+0.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JordanNoone
JordanNoone marked this pull request as ready for review September 6, 2026 04:10

@adamchalmers adamchalmers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls do not merge, hold for proper review

@adamchalmers

adamchalmers commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Please retarget this branch to deserialize command-line args for UoM via the kittycad-modeling-cmds crate (released in version 0.2.229) rather than implementing conversions between them.

@adamchalmers

Copy link
Copy Markdown
Contributor

kcmc v0.2.229 was released with the clap derive now so you can remove the crates.io patch.

Why does only volume need special treatment here, should this apply to all units? Genuine question.

@JordanNoone JordanNoone changed the title Align file and KCL volume unit parsing Parse KCL measurement units consistently Sep 7, 2026
Merge current main, remove the modeling-command Git patch, and require the published unit-parsing compatibility fixes. Retain main's locked 0.2.231 and refresh the Nix vendor hash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Volume unit enums disagree: zoo kcl volume accepts mm3, zoo file volume rejects it

2 participants