Skip to content

proposal(peat-schema): add Kinematics & PositionError common types#1023

Merged
phornstein merged 9 commits into
mainfrom
feat/common-kinematics-position-error
Jul 7, 2026
Merged

proposal(peat-schema): add Kinematics & PositionError common types#1023
phornstein merged 9 commits into
mainfrom
feat/common-kinematics-position-error

Conversation

@phornstein

@phornstein phornstein commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds two new shared message types in common.proto and wires them onto Track and NodeState, plus fixes all QA findings, a flaky test, a supply-chain advisory, and proto file housekeeping.

Schema changes

  • Kinematics — velocity, heading, acceleration, vertical_speed (consolidates the track-specific Velocity message into a reusable common type)
  • PositionError — circular_error (CEP), linear_error (LEP), vertical_error (consolidates error fields previously inlined on TrackPosition)
  • common.proto: added Kinematics and PositionError messages
  • track.proto: added optional kinematics (field 12) and position_error (field 13) on Track; deprecated Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m
  • node.proto: added optional kinematics (field 8) and position_error (field 9) on NodeState (LWW-Register semantics)

QA fixes

  • NodeStateExt::merge() LWW gap (BLOCKER): the new kinematics and position_error fields are now copied in the newer-wins branch, restoring correct LWW-Register convergence for all merge() call sites
  • Validation for new fields (WARNING): validate_kinematics enforces velocity non-negative, heading 0–360, and is_finite() on all float fields; validate_position_error enforces non-negative and is_finite() on all three error fields
  • NaN coverage gap (WARNING): heading and all PositionError fields now have is_finite() guards — NaN/Inf can no longer slip past range checks

Flaky test fix

  • formation_handshake race condition: removed the oneshot channel + 50ms sleep synchronization in run_handshake_test. The iroh QUIC listener queues connections at the endpoint level from IrohTransport::new(), so the accept/connect synchronization was unnecessary and the sleep was insufficient under CI load

Supply chain

  • RUSTSEC-2026-0204: updated crossbeam-epoch 0.9.18 → 0.9.20 (invalid pointer dereference in fmt::Pointer for Atomic/Shared)

Proto file housekeeping

  • Renamed "CAP Protocol" → "Peat Protocol" in all 14 proto files that still carried the legacy name
  • Unified all proto schema versions to 0.5.0 (pre-1.0, reflecting that breaking changes may still occur before 1.0; versioning strategy relative to the crate release cadence is under team discussion)
  • Added missing Version: line to command.proto

Backward compatibility

  • All new fields are optional — existing consumers are unaffected
  • Deprecated fields retain their field numbers and types (ICD §4.2.1: MUST NOT remove or renumber)
  • Marked with [deprecated = true] per ICD §4.2.2
  • No wire-breaking changes

Migration path

Consumers should adopt Track.kinematics / Track.position_error and stop writing to the deprecated fields. The deprecated fields will remain in the schema indefinitely per ICD rules.

Test plan

  • cargo check --workspace passes
  • cargo test -p peat-schema passes (156 tests including 20 new validation tests)
  • cargo test -p peat-protocol passes (999 tests including 2 new merge tests)
  • cargo clippy -p peat-schema -p peat-protocol -- -D warnings clean
  • cargo deny check advisories passes
  • formation_handshake tests pass 15/15 under stress
  • Deprecated field warnings suppressed with #[allow(deprecated)] in validation/test code

phornstein and others added 2 commits July 6, 2026 15:08
Add two new common message types for reuse across the schema:
- Kinematics: velocity, heading, acceleration, vertical_speed
- PositionError: circular_error, linear_error, vertical_error

Wire both as optional fields on Track (fields 12-13) and
NodeState (fields 8-9, LWW-Register semantics). Deprecate the
now-superseded Track.velocity, TrackPosition.cep_m, and
TrackPosition.vertical_error_m fields per ICD §4.2.2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@peat-bot peat-bot 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.

Peat QA Review (SHA: fa9090e)

Scope

Schema-only additions (Kinematics, PositionError in common.proto), wired onto Track (fields 12, 13) and NodeState (fields 8, 9). Deprecates Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m. Minor version bumps across all protos. Corresponding Rust struct initializers in peat-protocol and peat-schema tests updated for the new fields.

Assessment

  • Backward compatibility (schema): field numbers 12/13 on Track and 8/9 on NodeState do not collide with existing fields; deprecated fields retain their numbers and types; new fields are optional. Wire compatibility preserved.
  • No FIPS-related crypto touched. No FFI/JNI/UniFFI/BLE surface changed. No consumer-specific references introduced.
  • Automerge round-trip is JSON-based on the full NodeState struct, so the new fields sync through automerge_backend/automerge_conversion automatically.
  • Most other NodeState { .. } construction sites already use ..Default::default(), so they compile without change.

Findings

[BLOCKER] NodeStateExt::merge() does not merge the new kinematics / position_error fields — breaks the LWW-Register semantics claimed in the PR description

peat-protocol/src/models/node.rs:310-323:

fn merge(&mut self, other: &NodeState) {
    let self_ts  = self.timestamp.as_ref().map(|t| t.seconds).unwrap_or(0);
    let other_ts = other.timestamp.as_ref().map(|t| t.seconds).unwrap_or(0);

    if other_ts > self_ts {
        self.position     = other.position;
        self.health       = other.health;
        self.phase        = other.phase;
        self.cell_id      = other.cell_id.clone();
        self.zone_id      = other.zone_id.clone();
        self.fuel_minutes = other.fuel_minutes;
        self.timestamp    = other.timestamp;
        // MISSING: self.kinematics = other.kinematics.clone();
        // MISSING: self.position_error = other.position_error.clone();
    }
}

The PR body states the new NodeState.kinematics / NodeState.position_error fields use "LWW-Register semantics". This function is the LWW-Register handler for NodeState (it's the trait-level merge used by callers in capability_aggregation.rs, coordinator.rs, role.rs, and by all state1.merge(&state2) sites in tests). Because the new fields aren't copied in the newer-wins branch, adopting a peer's newer state will silently drop the peer's kinematics and position_error — the local copy retains its previous (older) values. That is not LWW; it's a stale-data leak.

Note: the Automerge path serializes the whole struct and is unaffected, so state that flows only through Automerge sync will converge. But any call site using NodeStateExt::merge directly will lose these fields on convergence. Ship the schema change and the merge change together, or pull the fields out of NodeState for this PR and land them in a follow-up.

Fix: extend the newer-wins branch with

self.kinematics     = other.kinematics.clone();
self.position_error = other.position_error.clone();

and add a unit test alongside the existing merge-based tests in peat-protocol/src/models/node.rs that asserts both new fields are adopted from the newer state.

[WARNING] validate_track not extended for the new Kinematics / PositionError fields on Track

peat-schema/src/validation/track.rs:81-113. The existing validate_track_position bounds-checks the deprecated cep_m (>= 0.0) and range-checks lat/lon. The new Track.kinematics and Track.position_error are added to the message but not validated:

  • Kinematics.heading is documented 0-360 (0 = North) — no range check.
  • Kinematics.velocity is documented "Speed in m/s" — semantically non-negative, no check.
  • Kinematics.vertical_speed — no NaN/finite check (float fields, hardware or codec errors can produce non-finite values).
  • PositionError.circular_error / linear_error / vertical_error — semantically non-negative, no check (mirrors what cep_m already validates for the deprecated pathway).

Migration intent is that consumers stop writing to cep_m / vertical_error_m and switch to PositionError. If validators only enforce constraints on the deprecated fields, garbage values on the new fields pass validation once producers migrate. Extend validate_track_position (or add a sibling validate_track_kinematics / validate_track_position_error) to enforce the same invariants on the new fields that were being enforced on the old.

@phornstein

Copy link
Copy Markdown
Contributor Author

@kitplummer @rutheferd-du @ai-strong-gb Please contribute thoughts to these enhancements to peat-schema. This originated from the work on ATAK and realizing that nodes need to carry kinematic information that doesn't exist today. Not all nodes are dynamic (ground sensor, base station, etc...) but UxS are. This would prevent the need for a UxS to broadcast both a track and node info.

@ai-strong-gb ai-strong-gb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

+1 on deprecating velocity in favor of Kinematics.

This aligns the schema with how ABI tradecraft and government track standards actually model entity motion. CoT's <track> element already bundles speed and course as a composite, and NFFI (STANAG 5527) groups bearing, speed, and inclination together under positionalData, motion state is inherently a structured concept, not a standalone scalar.

From an ABI perspective, tracks are time-ordered sequences of complete motion states, not just speed readings. Downstream consumers need position + direction + speed + acceleration as a unified object to do motion abstraction (loitering detection, route deviation, pattern-of-life analysis, etc.). A flat velocity field forces every consumer to reconstruct that composite from scattered fields, and it's semantically ambiguous.

Kinematics as a message type gives us:

  • Named, typed fields that map cleanly to different sensor observables (GPS ground track, radar radial velocity, IMU acceleration)
  • A single composable unit for the TAK bridge (CoT <track>) and future NATO interop (NFFI/STANAG mapping)
  • A self-contained object that edge AI and track correlation can reason over directly

Comment thread peat-schema/proto/cell.proto
Comment thread peat-schema/proto/hierarchy.proto
Comment thread peat-schema/src/validation/track.rs
@kitplummer

Copy link
Copy Markdown
Collaborator

@phornstein Great stuff! Makes good sense, agree with this movement. Note the QA Review findings must also be resolved.

…sitionError

NodeStateExt::merge() was not copying the new kinematics and
position_error fields in the LWW newer-wins branch, silently dropping
peer state on convergence. Track validation had no constraints on the
new fields, allowing garbage values once producers migrate off the
deprecated paths.

Fixes both peat-bot QA findings on #1023.

@peat-bot peat-bot 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.

Peat QA Review (SHA: 184e818)

Scope

Incremental review of the fix commit that addresses two prior QA findings on the Kinematics / PositionError schema proposal:

  • NodeStateExt::merge() now copies the new kinematics and position_error fields in the LWW newer-wins branch (peat-protocol/src/models/node.rs:322-323), with two new tests: test_node_state_merge_kinematics_and_position_error and test_node_state_merge_older_does_not_overwrite_kinematics.
  • validate_track now validates Track.kinematics and Track.position_error via new validate_kinematics / validate_position_error helpers (peat-schema/src/validation/track.rs:122-173), with 9 new tests.

Assessment

  • Prior [BLOCKER] (merge LWW gap) — RESOLVED. merge() now copies both new fields when other_ts > self_ts; tests exercise both the newer-wins path and the equal-timestamp non-overwrite path.
  • Prior [WARNING] (missing validation on new fields) — PARTIALLY RESOLVED. Range and negative checks were added, but NaN coverage is incomplete on Kinematics.heading and on all three PositionError fields (see finding below).
  • Schema wire compatibility, FIPS posture, FFI/BLE surface, and consumer-name discipline are unaffected by this commit (same as prior review).

Findings

[WARNING] Incomplete NaN coverage in the new validators — Kinematics.heading and all PositionError fields silently accept NaN

peat-schema/src/validation/track.rs:122-173. The prior review flagged missing NaN/finite checks with the rationale that "hardware or codec errors can produce non-finite values." The fix adds is_finite() checks for velocity, acceleration, and vertical_speed, but the pattern is inconsistent:

  • Kinematics.heading (line 134): checked only as heading < 0.0 || heading > 360.0. In Rust, all comparisons against NaN return false, so a NaN heading passes both range checks. There is no follow-up is_finite() guard. NaN heading slips through validation.
  • PositionError.circular_error, linear_error, vertical_error (lines 154, 160, 166): checked only as < 0.0. Again NaN < 0.0 is false, so all three fields silently accept NaN. No is_finite() guard anywhere in validate_position_error.

The existing test_kinematics_nan_velocity and test_kinematics_infinite_vertical_speed tests pass because those fields' is_finite() checks catch the problem — but there is no analogous test for test_kinematics_nan_heading or any NaN case in PositionError, so the gap is not test-visible either.

Fix: add an is_finite() guard to heading and to each PositionError field, mirroring the pattern already applied to velocity:

// in validate_kinematics, before the heading range check
if !kin.heading.is_finite() {
    return Err(ValidationError::InvalidValue(
        "kinematics.heading must be finite".to_string(),
    ));
}
// in validate_position_error, for each field
if !pe.circular_error.is_finite() {
    return Err(ValidationError::InvalidValue(
        "position_error.circular_error must be finite".to_string(),
    ));
}
// ...same for linear_error, vertical_error

Add corresponding NaN tests (test_kinematics_nan_heading, test_position_error_nan_circular, etc.) so the invariant is enforced by the test suite.

Note on Kinematics.velocity (line 123): the ordering < 0.0 check before is_finite() still catches NaN because NaN fails is_finite() on the second check — so velocity is safe, but the ordering is unintuitive; consider moving the is_finite() check first for consistency across fields. Not blocking on its own.

…n proto comments

Addresses the remaining peat-bot QA finding: heading and all
PositionError fields now have is_finite() guards so NaN/Inf cannot
slip past the range checks. Reorders velocity validation to check
finite-ness first for consistency across all fields.

Also renames the legacy "CAP Protocol" header comment to
"Peat Protocol" in 14 proto files (ADR-069 rename residue).

@peat-bot peat-bot 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.

Peat QA Review (SHA: a9c2ac0)

Scope

Incremental review of the follow-up fix commit on the Kinematics / PositionError schema proposal. This commit:

  • Completes NaN validation coverage in peat-schema/src/validation/track.rsis_finite() guards added for Kinematics.heading and for all three PositionError fields; Kinematics.velocity reordered to check finiteness first for consistency.
  • Adds matching NaN tests: test_kinematics_nan_heading, test_position_error_nan_circular, test_position_error_nan_linear, test_position_error_nan_vertical.
  • Renames CAP ProtocolPeat Protocol in the top-of-file comment header of 14 proto files (ADR-069 rename residue) and bumps their file-level version comments accordingly.

Assessment

  • Prior [BLOCKER] (merge LWW gap) — RESOLVED at peat-protocol/src/models/node.rs:322-323; two dedicated merge tests present.
  • Prior [WARNING] (missing validation on new fields) — RESOLVED; range, non-negativity, and NaN/finite checks are now uniformly enforced across Kinematics (velocity, heading, acceleration, vertical_speed) and PositionError (circular, linear, vertical) with matching test coverage.
  • Prior [WARNING] (incomplete NaN coverage on heading + PositionError) — RESOLVED at peat-schema/src/validation/track.rs:134,159,170,181 with corresponding NaN tests in the same file.
  • Schema wire compatibility unchanged: new fields remain optional; field numbers 8/9 (NodeState) and 12/13 (Track) do not collide; deprecated fields keep their numbers and types.
  • No FIPS-covered crypto touched. No FFI / JNI / UniFFI / BLE / Iroh / Automerge sync semantics changed. No consumer-specific references introduced — the rename removes stale CAP Protocol strings from comments only, which is aligned with ADR-069 and the repo-level generic-substrate rule.
  • Comment-only proto changes with version-bump metadata do not alter descriptor bytes; downstream prost codegen output is unaffected.

Findings

No findings.

Fixes cargo-deny Supply Chain CI failure caused by an invalid pointer
dereference in crossbeam-epoch's fmt::Pointer impl for Atomic/Shared.
All 20 proto files now carry `Version: 0.9.0-rc.29`, matching the
workspace crate version. Removes the divergent 1.1.0/2.1.0 scheme
that had drifted from the actual release cadence. Also adds the
missing version line to command.proto.

@peat-bot peat-bot 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.

Peat QA Review (SHA: 0500b76)

Scope

Incremental review of the crossbeam-epoch supply-chain bump on top of the prior clean state at a9c2ac0. This commit changes only Cargo.lock, updating crossbeam-epoch from 0.9.18 to 0.9.20 to address RUSTSEC-2026-0204 (invalid pointer dereference in fmt::Pointer for Atomic/Shared). The prior schema work on Kinematics / PositionError (common.proto, Track, NodeState, NodeStateExt::merge, validate_track) was already reviewed clean at a9c2ac0 and is unchanged in this push.

Assessment

  • Prior [BLOCKER] (merge LWW gap on new fields) — RESOLVED at 184e818 (peat-protocol/src/models/node.rs:322-323); merge tests present. Unchanged in this push.
  • Prior [WARNING] (missing validation on new fields) — RESOLVED at 184e818 with the validate_kinematics / validate_position_error helpers. Unchanged in this push.
  • Prior [WARNING] (incomplete NaN coverage) — RESOLVED at a9c2ac0 with is_finite() guards on heading and all three PositionError fields, plus matching NaN tests. Unchanged in this push.
  • New change (crossbeam-epoch 0.9.18 → 0.9.20) — patch-level bump within the same minor line, addressing a legitimate RUSTSEC advisory. crossbeam-epoch is not a cryptographic primitive (FIPS posture unaffected), not FFI/JNI/UniFFI/BLE surface, and not part of Automerge sync or Iroh discovery paths. Cargo.lock-only diff; no source changes required.
  • Schema wire compatibility, FIPS crypto set, FFI/BLE surface, Automerge sync semantics, Iroh peer discovery, OTA/A/B partition invariants, and consumer-name discipline are all unaffected by this commit.

Findings

No findings.

@peat-bot peat-bot 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.

Peat QA Review (SHA: f598266)

Scope

Incremental review of a comment-only unification commit on top of the prior clean state at 0500b76. This push unifies the // Version: header comment in all 20 peat-schema/proto/*.proto files to 0.9.0-rc.29 (matching the workspace crate version), and adds a missing version comment line to command.proto. Removes the drifted 1.0.0 / 1.1.0 / 2.0.0 / 2.1.0 scheme from the file-level comments. No .proto message definitions, field numbers, field types, options, or reserved ranges change in this push. All previously reviewed schema work on Kinematics / PositionError (common.proto, Track, NodeState, NodeStateExt::merge, validate_track) is unchanged.

Assessment

  • Prior [BLOCKER] (merge LWW gap on new fields) — RESOLVED at 184e8187 (peat-protocol/src/models/node.rs:322-323), with dedicated merge tests. Unchanged in this push.
  • Prior [WARNING] (missing validation on new fields) — RESOLVED at 184e8187 via validate_kinematics / validate_position_error. Unchanged in this push.
  • Prior [WARNING] (incomplete NaN coverage on heading and PositionError) — RESOLVED at a9c2ac0 with is_finite() guards and matching NaN tests. Unchanged in this push.
  • Prior clean review at 0500b76 (crossbeam-epoch supply-chain bump) — unchanged.
  • New change (proto file version comment unification) — comment-only edit. Comments in .proto files are stripped by protoc and do not appear in generated descriptor bytes or generated Rust; prost codegen output is unaffected. Wire compatibility, message layout, field numbers, and deprecation markers are all preserved verbatim.
  • The // Version: 2.1.0 — abstract hierarchy vocabulary per ADR-066 annotation on cell.proto and hierarchy.proto is replaced with the plain 0.9.0-rc.29 line, dropping the inline ADR-066 pointer from those two file headers. ADR-066 remains authoritative in docs/adr/ and the substantive tier 1 of 4 / four-tier body comments in those files are preserved, so this is a minor prose loss with no schema or behavior impact — not actionable.
  • No FIPS-covered cryptographic primitive touched. No FFI / JNI / UniFFI / BLE / Iroh / Automerge sync / OTA / A/B partition surface changed. No consumer-specific references introduced — the version strings are neutral crate metadata.
  • Schema backward-compatibility posture unchanged: new Kinematics / PositionError fields on Track (12, 13) and NodeState (8, 9) remain optional; deprecated Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m retain their field numbers and types.

Findings

No findings.

Sets all 20 proto files to a single schema version 0.5.0, reflecting
that the wire format may still have breaking changes before 1.0.
Removes the divergent 1.1.0/2.1.0 split and adds the missing version
line to command.proto. Schema versioning strategy relative to the
crate release cadence is under team discussion.

@peat-bot peat-bot 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.

Peat QA Review (SHA: 0447385)

Scope

Incremental review of the proto version-comment unification commit on top of the prior clean state at f598266. This push reverts the prior 0.9.0-rc.29 version-header value and replaces it with a single 0.5.0 schema version across all 20 peat-schema/proto/*.proto files. Rationale (per commit message 04473857): the wire format may still have breaking changes before 1.0, and schema versioning strategy relative to the crate release cadence is under team discussion. The previously reviewed schema work on Kinematics / PositionError (common.proto, Track, NodeState, NodeStateExt::merge, validate_track) and the crossbeam-epoch supply-chain bump are unchanged.

Assessment

  • Prior [BLOCKER] (merge LWW gap on new fields) — RESOLVED at 184e8187 (peat-protocol/src/models/node.rs:322-323), with dedicated merge tests. Unchanged in this push.
  • Prior [WARNING] (missing validation on new fields) — RESOLVED at 184e8187 via validate_kinematics / validate_position_error. Unchanged in this push.
  • Prior [WARNING] (incomplete NaN coverage on heading and PositionError) — RESOLVED at a9c2ac0 with is_finite() guards and matching NaN tests. Unchanged in this push.
  • Prior clean review at 0500b76 (crossbeam-epoch 0.9.18 → 0.9.20, RUSTSEC-2026-0204) — unchanged.
  • Prior clean review at f598266 (proto version comment unification to 0.9.0-rc.29) — value replaced by 0.5.0 in this push.
  • New change (proto version comment set to 0.5.0) — comment-only edit. .proto comments are stripped by protoc and do not appear in generated descriptor bytes or generated Rust; prost codegen output is unaffected. Wire compatibility, message layout, field numbers, and deprecation markers are all preserved verbatim. Choosing a pre-1.0 schema version to signal that breaking wire changes are still possible before stabilization is a reasonable prose choice and matches the field-level state (new fields still being added, deprecated fields kept for compat) — not actionable.
  • No FIPS-covered cryptographic primitive touched. No FFI / JNI / UniFFI / BLE / Iroh / Automerge sync / OTA / A/B partition surface changed. No consumer-specific references introduced — the version strings are neutral crate metadata.
  • Schema backward-compatibility posture unchanged: new Kinematics / PositionError fields on Track (12, 13) and NodeState (8, 9) remain optional; deprecated Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m retain their field numbers and types.

Findings

No findings.

The test helper signalled readiness via a oneshot channel before
accept() was called, then papered over the scheduling gap with a
50ms sleep. Under CI load the sleep was insufficient, causing
sporadic timeout failures.

The iroh endpoint's QUIC listener is active from IrohTransport::new()
— incoming connections queue at the transport level regardless of
whether accept() has been called. Removed the oneshot channel and
sleep entirely; the responder task now runs concurrently with connect
and dequeues the connection whenever it is scheduled.

@peat-bot peat-bot 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.

Peat QA Review (SHA: 599a095)

Scope

Incremental review of the formation_handshake test-helper simplification commit (599a0950) on top of the prior clean state at 0447385. This push removes the oneshot ready-signal + 50 ms sleep from run_handshake_test in peat-protocol/src/network/formation_handshake.rs (test module), replacing them with a doc comment explaining that iroh's QUIC listener is active as soon as IrohTransport::new() returns and queues incoming connections at the transport level, so no accept()-readiness signal is needed. All previously reviewed schema work on Kinematics / PositionError (common.proto, Track, NodeState, NodeStateExt::merge, validate_track), the crossbeam-epoch supply-chain bump, and the proto version-comment unification to 0.5.0 are unchanged.

Assessment

  • Prior [BLOCKER] (merge LWW gap on new fields) — RESOLVED at 184e8187 (peat-protocol/src/models/node.rs:322-323), with dedicated merge tests. Unchanged in this push.
  • Prior [WARNING] (missing validation on new fields) — RESOLVED at 184e8187 via validate_kinematics / validate_position_error. Unchanged in this push.
  • Prior [WARNING] (incomplete NaN coverage on heading and PositionError) — RESOLVED at a9c2ac0 with is_finite() guards and matching NaN tests. Unchanged in this push.
  • Prior clean reviews at 0500b76 (crossbeam-epoch 0.9.18 → 0.9.20, RUSTSEC-2026-0204), f598266 (proto version-comment unification, superseded), and 0447385 (proto version pinned to 0.5.0) — all unchanged in this push.
  • New change (formation_handshake test-helper simplification) — scope is confined to #[cfg(test)] mod tests in peat-protocol/src/network/formation_handshake.rs. Removes use tokio::sync::oneshot; and the ready-signal handshake (ready_tx/ready_rx) plus the 50 ms tokio::time::sleep. run_handshake_test now spawns the responder task and immediately drives the initiator's connect(). The doc comment on the helper documents the invariant this relies on: iroh's endpoint is listening from IrohTransport::new() onward and queues incoming connections at the QUIC transport layer regardless of whether accept() has been called yet, so the responder task's accept() will dequeue the pending connection whenever it is scheduled. Tests remain #[serial]-gated so endpoint churn stays sequential.
  • No production code path changed by this commit — the transport-layer behavior being relied on (endpoint listens on construction; queues at the QUIC layer until accept()) is iroh's documented design, not a new assumption introduced here. Race-condition risk is against test flakiness, not runtime correctness.
  • No FIPS-covered cryptographic primitive touched. No FFI / JNI / UniFFI / BLE / OTA / A/B partition surface changed. Automerge sync semantics and Iroh peer discovery behavior are untouched (the change is purely in test-helper orchestration). No consumer-specific references introduced.
  • Schema backward-compatibility posture unchanged: new Kinematics / PositionError fields on Track (12, 13) and NodeState (8, 9) remain optional; deprecated Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m retain their field numbers and types.

Findings

No findings.

@peat-bot peat-bot 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.

Peat QA Review (SHA: 599a095)

Scope

Re-trigger of the incremental review at head 599a095 (no new commits since the prior QA pass at this SHA). The PR body was updated but no code changed. Cumulative diff since main covers:

  • peat-schema/proto/common.proto: new Kinematics and PositionError messages.
  • peat-schema/proto/track.proto: added optional common.v1.Kinematics kinematics = 12; and optional common.v1.PositionError position_error = 13; on Track; marked Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m as [deprecated = true] while retaining their field numbers and types.
  • peat-schema/proto/node.proto: added optional common.v1.Kinematics kinematics = 8; and optional common.v1.PositionError position_error = 9; on NodeState.
  • peat-protocol/src/models/node.rs: NodeStateExt::merge() newer-wins branch copies the two new fields (self.kinematics = other.kinematics; self.position_error = other.position_error;); two dedicated merge tests cover both the newer-wins path and the older-does-not-overwrite path.
  • peat-schema/src/validation/track.rs: new validate_kinematics and validate_position_error helpers with is_finite() guards on every float field plus range/non-negativity constraints; 20 new validation tests including NaN cases for heading and all three PositionError fields.
  • peat-protocol/src/network/formation_handshake.rs (test module): run_handshake_test no longer uses a oneshot ready-signal + 50 ms sleep; the responder task now runs concurrently with the initiator's connect(), relying on the documented iroh transport-layer behavior that the endpoint listens from IrohTransport::new() and queues incoming connections at the QUIC layer regardless of whether accept() has been called.
  • Cargo.lock: crossbeam-epoch 0.9.18 → 0.9.20 for RUSTSEC-2026-0204.
  • Proto file header comments: CAP ProtocolPeat Protocol in 14 files; version comment unified to 0.5.0 in all 20 proto files (pre-1.0, per commit rationale); missing version line added to command.proto.

Assessment

  • Prior [BLOCKER] (NodeStateExt::merge() LWW gap on new fields) — RESOLVED at 184e8187 (peat-protocol/src/models/node.rs:322-323); merge tests exercise both directions of the LWW ordering. Verified against current tree.
  • Prior [WARNING] (missing validation on new Kinematics / PositionError fields) — RESOLVED at 184e8187 via validate_kinematics / validate_position_error, called from validate_track. Verified against current tree.
  • Prior [WARNING] (incomplete NaN coverage on heading and all PositionError fields) — RESOLVED at a9c2ac0 with is_finite() guards and matching NaN tests. Verified against current tree at peat-schema/src/validation/track.rs lines 123/134/159/170/181.
  • Prior clean review at 0500b76 (crossbeam-epoch 0.9.18 → 0.9.20, RUSTSEC-2026-0204) — unchanged.
  • Prior clean review at f598266 (proto version comment unification, superseded by 0.5.0) — unchanged.
  • Prior clean review at 0447385 (proto version pinned to 0.5.0, pre-1.0 signal) — unchanged.
  • Prior clean review at 599a095 (formation_handshake test-helper simplification) — unchanged; this re-trigger reflects a PR title/description edit only.
  • PR title and current description accurately describe the diff: schema additions, backward-compat posture (new fields optional, deprecated fields retained with their numbers), FIPS surface untouched (no crypto primitive changes), FFI/BLE/UniFFI/JNI surface untouched, Automerge and Iroh sync semantics untouched. The pre-merge checklist boxes are consistent with the state of the tree. Nothing in the updated description introduces a new claim requiring re-validation.
  • No FIPS-covered cryptographic primitive touched. No FFI / JNI / UniFFI / BLE / OTA / A/B partition surface changed. Automerge sync semantics and Iroh peer discovery behavior remain untouched. No consumer-specific references introduced — the CAP ProtocolPeat Protocol comment rename removes stale legacy strings per ADR-069 and the generic-substrate rule in this repo's CLAUDE.md.
  • Schema backward compatibility preserved: new Kinematics / PositionError fields on Track (12, 13) and NodeState (8, 9) are optional; deprecated Track.velocity, TrackPosition.cep_m, TrackPosition.vertical_error_m retain their field numbers and types (ICD §4.2.1 / §4.2.2).
  • Test-surface tier is appropriate for the changed surface: this PR adds fields to internal protobuf messages consumed inside the peat-protocol crate; no new UniFFI export, JNI extern fn, Translator trait impl, or fanout registration is introduced, so no wrapper-tier E2E is required by the surface-coverage rule.

Findings

No findings.

@phornstein phornstein merged commit dd5e798 into main Jul 7, 2026
24 checks passed
@phornstein phornstein deleted the feat/common-kinematics-position-error branch July 7, 2026 18:49
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.

4 participants