Skip to content

fix: preserve F10 rule IDs for non-finite quaternions - #420

Merged
Blogbotana merged 1 commit into
Apibim-com:masterfrom
axisrow:bug/f10-quaternion-diagnostics
Oct 26, 2026
Merged

Blogbotana merged 1 commit into
Apibim-com:masterfrom
axisrow:bug/f10-quaternion-diagnostics

Conversation

@axisrow

@axisrow axisrow commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

What this changes

Non-finite quaternion components already reject with NON_FINITE_NUMBER, but two model paths omit the F10 domain rule. Attach DV-ATTR-003 in Placement::validate_orientation and quaternion encoding in UpdateElement::content_fingerprint, so existing diagnostic propagation can identify the numeric-attribute violation.

This isolates the two unpublished annotations found while auditing the legacy F10 checkout against merged #404. It is rebased directly onto current master 65728b0 and has no dependency on the grid PRs. Refs #148; follows #404.

How you verified it

  • Regression test failed before the fix because rule_id was None.
  • The test covers NaN and positive/negative infinity in every quaternion component through direct orientation validation, update argument validation and direct fingerprint encoding.
  • cargo test -p beamcraft-model --offline --locked: all 72 tests passed; doc-tests passed.
  • cargo clippy -p beamcraft-model --all-targets --offline --locked -- -D warnings: passed.
  • cargo fmt --all -- --check and diff whitespace checks: passed.
  • Applied the existing fix(tests): use the #406 command names in the levels test #418 command-name correction to the merged F10 copy acceptance test; the focused actual-binary test passed. CI is running again for ffb3922.
  • fix(tests): use the #406 command names in the levels test #418 has now merged. The redundant command-name commit was removed while rebasing onto 65728b0; this PR again changes only quaternion diagnostics, its regression test and technical note. CI is running for 45a458e.
  • Full workspace/platform CI is pending; keep draft until required checks pass.

Never-crash

Two error annotations only in production code. No new panic/unwrap/expect, input-derived indexing, allocations or unsafe. Existing rejection code and field remain unchanged, and accepted command fingerprints are unchanged.

Provenance

Contributor-original MPL-2.0 code and technical note. No dependencies, third-party code or assets added.

Anything a reviewer should look at first

F10 DV-ATTR-003 covers non-finite numeric attributes. Both public entry points must preserve this rule even when fingerprint encoding is called without prior argument validation. This follow-up does not claim completion of the broader F10 acceptance gate.

@axisrow
axisrow requested a review from Blogbotana October 26, 2026 15:33
@axisrow
axisrow force-pushed the bug/f10-quaternion-diagnostics branch from ffb3922 to 45a458e Compare October 26, 2026 16:58
@axisrow
axisrow marked this pull request as ready for review October 26, 2026 17:15

@axisrow axisrow left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Approving

Both public entry points now carry DV-ATTR-003: Placement::validate_orientation and the quaternion encoding path in UpdateElement::content_fingerprint. The regression test sweeps NaN and both infinities through every component across direct validation, argument admission and fingerprint encoding, asserting code, field and rule_id. The rule id matches the existing DV-ATTR-003 usage on master; finite admission and accepted fingerprints stay unchanged. No changes requested.

@Blogbotana
Blogbotana merged commit e4a1690 into Apibim-com:master Oct 26, 2026
32 checks passed
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.

2 participants