Angle violates the PartialEq/PartialOrd consistency contract, and rtc_frames_per_second is narrowed from a double to u8
Context: API Makeathon participant. Found while reviewing the shared types and WebRTC metrics.
1. Angle PartialEq/PartialOrd are inconsistent
modeling-cmds/src/shared.rs:963 derives structural PartialEq, so equality compares both the unit tag and the numeric value. But PartialOrd at shared.rs:1030 is implemented by converting mixed units before comparing.
So Angle::from_degrees(0.0) and Angle::from_radians(0.0) compare as Ordering::Equal under partial_cmp yet are not equal under ==. A partial order must keep == and Some(Equal) consistent. Generic code that relies on that contract (sorting, dedup, range/interval checks, BTreeMap-style logic) will disagree about two physically identical angles: a <= b && b <= a is true while a == b is false.
2. rtc_frames_per_second cannot hold a fractional or high frame rate
modeling-cmds/src/websocket.rs:502 models rtc_frames_per_second as Option<u8>. The WebRTC Stats definition for framesPerSecond is a double. Fractional rates are valid and common (29.97), and u8 also caps the value at 255.
A Rust client cannot represent 29.97 FPS or 256 FPS, and deserializing a standards-shaped fractional value fails outright instead of preserving the metric.
Concrete failure
Deduplicating a set of angles, or comparing them for equality after a units-normalizing sort, produces contradictory results. Separately, deserializing {"rtc_frames_per_second": 29.97} into ClientMetrics fails.
Verify
Assert both Angle::from_degrees(0.0) == Angle::from_radians(0.0) (false) and Angle::from_degrees(0.0).partial_cmp(&Angle::from_radians(0.0)) == Some(Ordering::Equal) (true). Deserialize {"rtc_frames_per_second":29.97} into ClientMetrics: serde rejects the floating-point value.
Suggested fix
Make Angle's PartialEq unit-aware (compare after conversion), so it agrees with PartialOrd. Change rtc_frames_per_second to Option<f64> to match the WebRTC Stats double.
Environment
Reviewed against the current main of KittyCAD/modeling-api.
Angleviolates thePartialEq/PartialOrdconsistency contract, andrtc_frames_per_secondis narrowed from a double tou8Context: API Makeathon participant. Found while reviewing the shared types and WebRTC metrics.
1.
AnglePartialEq/PartialOrd are inconsistentmodeling-cmds/src/shared.rs:963derives structuralPartialEq, so equality compares both the unit tag and the numeric value. ButPartialOrdatshared.rs:1030is implemented by converting mixed units before comparing.So
Angle::from_degrees(0.0)andAngle::from_radians(0.0)compare asOrdering::Equalunderpartial_cmpyet are not equal under==. A partial order must keep==andSome(Equal)consistent. Generic code that relies on that contract (sorting, dedup, range/interval checks,BTreeMap-style logic) will disagree about two physically identical angles:a <= b && b <= ais true whilea == bis false.2.
rtc_frames_per_secondcannot hold a fractional or high frame ratemodeling-cmds/src/websocket.rs:502modelsrtc_frames_per_secondasOption<u8>. The WebRTC Stats definition forframesPerSecondis adouble. Fractional rates are valid and common (29.97), andu8also caps the value at 255.A Rust client cannot represent 29.97 FPS or 256 FPS, and deserializing a standards-shaped fractional value fails outright instead of preserving the metric.
Concrete failure
Deduplicating a set of angles, or comparing them for equality after a units-normalizing sort, produces contradictory results. Separately, deserializing
{"rtc_frames_per_second": 29.97}intoClientMetricsfails.Verify
Assert both
Angle::from_degrees(0.0) == Angle::from_radians(0.0)(false) andAngle::from_degrees(0.0).partial_cmp(&Angle::from_radians(0.0)) == Some(Ordering::Equal)(true). Deserialize{"rtc_frames_per_second":29.97}intoClientMetrics: serde rejects the floating-point value.Suggested fix
Make
Angle'sPartialEqunit-aware (compare after conversion), so it agrees withPartialOrd. Changertc_frames_per_secondtoOption<f64>to match the WebRTC Statsdouble.Environment
Reviewed against the current
mainof KittyCAD/modeling-api.