Skip to content

feat: ADR-068 Phase 1 — rename platform→node across the peat workspace (schema + consumers)#969

Merged
kitplummer merged 7 commits into
mainfrom
feat/968-phase1-schema-node-rename
Jun 6, 2026
Merged

feat: ADR-068 Phase 1 — rename platform→node across the peat workspace (schema + consumers)#969
kitplummer merged 7 commits into
mainfrom
feat/968-phase1-schema-node-rename

Conversation

@kitplummer

Copy link
Copy Markdown
Collaborator

Phase 1 of ADR-068 (#967) / epic #968: converge the base unit on Node, removing platform from the substrate vocabulary. This is the whole-peat-repo phase (peat-schema + peat-protocol + peat-transport + peat-ffi + peat-persistence + examples) — they share one compilation unit, so the schema rename and its in-repo consumers land together.

Renames

  • platform_idnode_id, platform_typenode_type, target_platformstarget_nodes, source_platform(s)source_node(s) across .proto + Rust.
  • DEVICE_TYPE_SENSOR_PLATFORMDEVICE_TYPE_SENSOR_NODE (consistent with the adjacent DEVICE_TYPE_RELAY_NODE).
  • FFI: PlatformStatusNodeStatus, PlatformInfoNodeInfo, collections::PLATFORMSNODES.
  • Removed vestigial as Platform{Config,State,Store} compat re-exports.

⚠️ Breaking / cross-phase coordination

  • Wire-breaking (proto field renames) — pre-1.0 clean break, as ADR-066 did.
  • Collection name "platforms""nodes" (the storage key for node docs). peat-node (Phase 4) must rename its collection to "nodes" in lockstep, or sync mismatches across the inter-phase window. Existing "platforms"-collection data is dropped (pre-1.0 clean break).
  • Breaks peat-atak-plugin: the FFI PlatformStatus/PlatformInfo/PLATFORMS rename changes generated bindings → coordinated peat-atak-plugin PR required (Phase 5).

Verification

Whole peat workspace builds (0 errors). Lib tests pass: peat-schema 147, peat-protocol 1012, peat-ffi 57. Only platform left in-repo: role.proto's role-label prose ("Strike/Support platform") and ONNX "cross-platform" — both intentionally preserved per ADR-068.

Next: Phase 3 peat-mesh (HierarchyLevel::Platform → Node), Phase 4 peat-node (Platform message/RPCs + collection name), Phase 5 consumers + CI grep gate.

platform_id->node_id, platform_type->node_type, target_platforms->target_nodes,
source_platform(s)->source_node(s) across all .proto; DEVICE_TYPE_SENSOR_PLATFORM
->DEVICE_TYPE_SENSOR_NODE; src/ field accesses + type_registry + validation + doc
examples updated. Wire-breaking. role.proto role-label prose left per ADR-068.
peat-schema builds; 147 tests pass. Workspace consumers renamed in follow-on
commits (same PR) so the peat workspace compiles.
…nsumers

Completes the peat-repo half of ADR-068 (epic #968) so the workspace compiles:
peat-protocol, peat-transport, peat-ffi, peat-persistence, examples.

- platform_id->node_id, platform_type->node_type, target_platforms->target_nodes,
  source_platform(s)->source_node(s); PlatformStatus->NodeStatus,
  PlatformInfo->NodeInfo (FFI types — breaks peat-atak-plugin bindings, coordinated
  Phase-5 follow-up); collections::PLATFORMS->NODES and its wire value
  "platforms"->"nodes" (storage-key change — peat-node Phase 4 must match).
- Removed vestigial `as Platform{Config,State,Store}` compat re-exports.
- Protected genuine computing-platform terms (ONNX cross-platform) and role.proto
  role-label prose per ADR-068.

Wire- and FFI-breaking (pre-1.0 clean break). Workspace builds; peat-schema 147,
peat-protocol 1012, peat-ffi 57 lib tests pass.

@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: c00f8ad)

ADR-068 Phase 1 — mechanical platform → node rename across peat-schema, peat-protocol, peat-transport, peat-ffi, peat-persistence, and examples. The rename is wire-breaking on protobuf + JNI surfaces, and the PR acknowledges Phase 3 (peat-mesh) / Phase 4 (peat-node) / Phase 5 (peat-atak-plugin) coordination requirements. Findings below.

[BLOCKER]

Ontology duplicate-id registration is silently brokenpeat-schema/src/ontology.rs:149-158. The rename produced two add_concept(Concept::new("node", "Node", ConceptCategory::Entity)...) calls. Ontology::add_concept uses HashMap::insert(concept.id.clone(), concept), so the second call overwrites the canonical top-level "node" concept with one declared with_parent("node") — a self-parent. Result: (a) the top-level node concept (and its description) is lost; (b) the surviving "node" concept self-loops in is_subtype_ofis_subtype_of("node", "<anything-not-equal-and-not-reached-by-parents>") recurses into is_subtype_of("node", same) indefinitely (peat-schema/src/ontology.rs:113-127 has no visited-set / cycle guard). Existing tests pass only because every assertion either short-circuits via reflexivity (is_subtype_of("node","node") → true on line 114) or queries from a non-self-parented concept (is_subtype_of("uav","node") reaches "node" via parent and returns true on the equality check). Fix: delete the second add_concept block (the old "platform" registration) and let uav/ugv/soldier_system keep .with_parent("node") pointing at the surviving canonical concept; restore the canonical node description ("A node in the Peat Protocol network…").

Cross-crate API references rewritten against an unchanged peat-btle pinpeat-ffi/src/lib.rs:103,1528,1570,1624. The diff rewrites peat_btle::platform::android::AndroidAdapter, peat_btle::platform::linux::BluerAdapter, and peat_btle::BleError::PlatformError to peat_btle::node::* / BleError::NodeError. These are symbols owned by the external peat-btle crate; the workspace pin in Cargo.toml:300 is unchanged at >=0.4.0, <0.4.1 and Cargo.lock still resolves peat-btle = 0.4.0. The PR explicitly enumerates Phases for peat-mesh/peat-node/peat-atak-plugin but does not list peat-btle, and peat-ffi's default = ["sync"] does NOT include bluetooth, so cargo check -p peat (the verification command in CLAUDE.md) never compiles the cfg-gated bluetooth code paths. The "Whole peat workspace builds (0 errors)" claim in the PR body therefore does not cover Android/Linux+--features bluetooth builds. If peat-btle 0.4.0 has not been forward-aliased to expose node::* and BleError::NodeError, this PR silently breaks the Android JNI BLE build and the Linux BluerAdapter path. Before merge: either bump the peat-btle pin to a release that exposes the renamed surface, or back out the peat_btle::* path rewrites in peat-ffi and gate them on a future peat-btle phase. Verify with cargo check -p peat-ffi --features bluetooth on linux and an android-target build.

[WARNING]

PR description contradicts the diff regarding role-label preservation — PR body states: "Only platform left in-repo: role.proto's role-label prose ('Strike/Support platform') and ONNX 'cross-platform' — both intentionally preserved per ADR-068." The diff at peat-schema/proto/role.proto:18-19 and peat-protocol/src/models/role.rs:23-24,52-54 renames Strike platformStrike node and Support platformSupport node. Either the rename violates the ADR-068 preservation directive, or the PR description is stale. Reconcile before merge so reviewers and downstream readers know whether to expect "platform" in role prose.

Schema documentation not updated in lockstep with wire renamepeat-schema/README.md:75,108,133,174, peat-schema/ICD.md:119,375, peat-schema/SCHEMAS.md:59,62,152,168. The Rust/Python/Java/protobuf code samples in README.md still use platform_type: "UAV" / setPlatformType("UAV") / string platform_type = 2;. ICD.md still asserts the validation rule on platform_type. After the wire rename, any consumer following these docs writes code that does not compile or interoperate. For a wire-breaking change, the documentation rename belongs in this same phase.

Composition metadata JSON keys silently rewritten on a wire-bearing fieldpeat-protocol/src/composition/constraint.rs:124-127,408-411. The renames "platform_count""node_count" and "limiting_platform""limiting_node" apply to JSON strings emitted into Capability::metadata_json, which flows through composition output to consumers via Automerge-replicated docs. The PR's breaking-change call-out covers proto field renames but does not mention these embedded-JSON keys; downstream parsers that consume metadata_json by key name will silently miss the renamed fields without a deserialization error. List these renames in the PR body's breaking-change block so consumer repos know to update parsers.

Renamed JNI surfaces lack bidirectional E2E coveragepeat-ffi/tests/jni_wrapper_integration.rs:516-519,528-531. getPlatformsJnigetNodesJni and publishPlatformJnipublishNodeJni are public JNI extern fn renames. The test diff only updates the symbol-existence audit table (scenario_native_method_table_audit) and a generic publish/get scenario that goes through publishDocumentJni/getDocumentJni against collection "nodes" — not through the renamed type-specific JNIs. Per the surface-tier coverage rule (peat-btle PR #57, peat-ffi PR #841), a modified JNI extern fn should ship an encode-via-wrapper + decode-via-wrapper round-trip exercising NodeInfo (e.g., publish a NodeInfo via publishNodeJni and read it back via getNodesJni). The gap may pre-date this PR, but a wire-breaking rename is the right moment to close it — symbol-table audits do not catch marshalling-shape drift.

[IDIOM]

Comment / test-name artifacts from mechanical sed — multiple sites carry leftover noise after the rename:

  • peat-schema/proto/node.proto:104// List of node/node IDs (was "platform/node IDs").
  • peat-schema/src/ontology.rs:21/// Physical entities (nodes, nodes).
  • peat-schema/src/ontology.rs:151 — description string "A node in the Peat Protocol network (node or system)".
  • peat-schema/src/ontology.rs:323-326 — test comments // Node is a Node / // UAV is a node and the assertion is_subtype_of("node","node") is now a trivial reflexive check duplicating the assertion on line 333 (loses the original "platform parents into node" coverage).
  • peat-schema/src/ontology.rs:344// node, node, uav, ugv, soldier_system.
    Clean up the prose so the next reader doesn't have to decode the rename history.

[ARCH]

Multi-repo coordination window for the "platforms""nodes" collection rename. The PR notes that peat-node Phase 4 must rename its collection to "nodes" in lockstep and that peat-atak-plugin Phase 5 must absorb the FFI ABI break, and concedes "Existing 'platforms'-collection data is dropped (pre-1.0 clean break)." This is the correct call-out, but the inter-phase window — between this PR merging and peat-node + peat-atak-plugin shipping — has nodes on different sides writing to different Automerge collections that will not merge (silent CRDT divergence, not a hard error). The architectural question is whether the rollout strategy should be (a) freeze peat-node / peat-atak-plugin until all phases land, (b) tag a coordinated release across repos, or (c) accept the dirty window with a documented downtime. This is ADR-territory and warrants human design review of the cross-repo rollout sequencing — not an inline fix in this PR. Escalating per the [ARCH] tag definition.

…Level::Node)

peat-mesh rc.35 ships the HierarchyLevel::Platform -> Node rename (peat-mesh#237,
ADR-068 Phase 3). The peat-transport TAK bridge in this branch already references
peat_mesh::HierarchyLevel::Node, so this lock bump (rc.31 -> rc.35, within the
existing >=0.9.0-rc.31, <0.9.1 pin) resolves the workspace build that was red on
the missing variant. No manifest change — range pin already admits rc.35.

Refs: #968

@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: 550cb22)

Scope: ADR-068 Phase 1 mass rename platformnode across peat-schema proto + Rust, peat-protocol, peat-transport, peat-ffi, peat-persistence, examples; plus peat-mesh bump rc.31→rc.35 for the HierarchyLevel::Platform→Node variant. Wire-breaking by design (pre-1.0 clean break, consistent with ADR-066).

Surface-tier coverage on the renamed UniFFI/JNI surface and the BLE Translator is in good shape: put_node_get_nodes_preserves_battery_and_heart, serialize_nodes_get_json_round_trips_through_parser, publish_json_inline_parser_extracts_battery_and_heart, the scenario_publish_get_roundtrip / scenario_native_method_table_audit JNI integration tests, and the peripheral_to_node / node_to_peripheral Translator round-trips all migrated together with the rename. BLE postcard wire shape (BlePeripheral, BlePosition, BleHealthStatus) is untouched — only the storage collection routing key changes, so M5Stack / WearOS firmware on the radio side keeps working. The local node vs Arc shadowing risk in publishNodeJni is correctly resolved by renaming the Arc binding to peat_node. No FIPS-primitive drift introduced.

[BLOCKER] In-repo Android example not updated; dual-peer functional test will break

examples/peat-ffi/examples/dual_test_peer.rs (the Linux/Pi side of the dual-peer BLE/QUIC functional test per the device infrastructure in CLAUDE.md — rPi 192.168.10.x + Android via ADB) is migrated in this PR to put_node / get_nodes and the "nodes" collection, but its Android counterpart in the same repo is left on the old surface:

  • examples/android-ble-test/app/src/main/java/com/defenseunicorns/peat/PeatJni.kt:177external fun getPlatformsJni(handle: Long): String
  • examples/android-ble-test/app/src/main/java/com/defenseunicorns/peat/PeatJni.kt:186external fun publishPlatformJni(handle: Long, platformJson: String): Boolean
  • examples/android-ble-test/app/src/main/java/com/defenseunicorns/peat/PeatJni.kt:503,510 — Kotlin wrappers getPlatformsJson() / publishPlatform(...) calling the above
  • examples/android-ble-test/app/src/main/java/com/defenseunicorns/peat/test/TestRunner.kt:370,385 — direct calls to PeatJni.publishPlatformJni / PeatJni.getPlatformsJni
  • examples/android-ble-test/app/src/main/java/com/defenseunicorns/peat/test/TestRunner.kt:360 — JSON payload publishes "platform_type": "HANDHELD"

The Rust JNI symbols those Kotlin externs resolve against are renamed to Java_..._getNodesJni / Java_..._publishNodeJni in peat-ffi/src/lib.rs (and the nativeInit / JNI_OnLoad registration tables only register the new names). Once the Android app loads the new libpeat.so, the two external fun declarations have nothing to bind to — Kotlin will throw UnsatisfiedLinkError the first time publishPlatformJni/getPlatformsJni is invoked, which is on the dual-peer test's hot path. Even before the link failure, "platform_type" in the JSON would be silently defaulted to "SOLDIER" by parse_node_publish_json (it unwrap_or("SOLDIER")s on the node_type key, so the rename causes a quiet semantic drop, not just a missing field).

The PR description explicitly carves out a Phase 5 follow-up PR for the sibling peat-atak-plugin repo on the same FFI rename, but does not address the in-repo Android example. Either land the Kotlin rename in this PR (the same rename mechanically: external fun publishNodeJni, external fun getNodesJni, publishNode(...), getNodesJson(), and "node_type" in the TestRunner JSON), or carve it out with a tracking issue and gate the dual-peer functional test on Phase 5. Shipping Phase 1 without it leaves the dual-peer QA infra in a broken state for the entire Phase-1→Phase-5 window.

[WARNING] peat-btle import path renamed without a dependency bump — verify 0.4.0 already exposes peat_btle::node

The PR rewrites every peat_btle::platform::{android,apple,linux,BleAdapter} import to peat_btle::node::{android,apple,linux,BleAdapter} and the only BleError variant referenced from BleError::PlatformError to BleError::NodeError, in peat-ffi/src/lib.rs (gated by target_os = "android" / target_os = "linux") and examples/peat-ble-test/src/main.rs (target_os = "macos" / "linux" / fallback StubAdapter). Cargo.lock only bumps peat-mesh (rc.31→rc.35); the peat-btle entry stays pinned at 0.4.0 from crates.io with the same checksum. The workspace pin in Cargo.toml is peat-btle = ">=0.4.0, <0.4.1" so the lock file genuinely admits exactly one version.

If peat-btle 0.4.0 already ships the node module + BleError::NodeError variant (i.e. an earlier, separately-released phase did the rename and the rc.31-era consumer code was using a deprecated platform re-export this PR is now migrating off), the build is fine. If 0.4.0 still only exposes platform and BleError::PlatformError, every Linux / Android / macOS target build will fail to compile, which the PR description's "workspace builds, 0 errors" claim would contradict.

Please confirm explicitly which peat-btle release introduced the rename and that 0.4.0 is on the right side of it. If 0.4.0 is the wrong side, this becomes a [BLOCKER] — bump the version and the pin together. CLAUDE.md's "no consumer-specific references" hard rule does not apply here (peat-btle is part of the substrate), but the Phase plan in the PR body skips peat-btle and that asymmetry is exactly what makes the dependency-state question load-bearing.

[WARNING] role.proto role-label prose changed despite ADR-068 / PR-description carve-out

peat-schema/proto/role.proto:

-  CELL_ROLE_STRIKE = 5;    // Strike platform - primary weapons
-  CELL_ROLE_SUPPORT = 6;   // Support platform - logistics/medical
+  CELL_ROLE_STRIKE = 5;    // Strike node - primary weapons
+  CELL_ROLE_SUPPORT = 6;   // Support node - logistics/medical

And the matching consumer prose in peat-protocol/src/models/role.rs:48-67 / function names (score_platform_for_rolescore_node_for_role, best_role_for_platformbest_role_for_node, score_platform_healthscore_node_health, RoleAssignment::platform_idnode_id).

Both this PR's description ("Only platform left in-repo: role.proto's role-label prose ('Strike/Support platform') and ONNX 'cross-platform' — both intentionally preserved per ADR-068") and the repo CLAUDE.md ("role.proto role-label prose left per ADR-068") explicitly state role.proto's "Strike/Support platform" prose is supposed to survive Phase 1 as a class-of-vehicle term distinct from the substrate-node vocabulary. The actual diff does the opposite.

Pick a direction and reconcile: either revert the role.proto enum comments + the matching Rust doc-prose in models/role.rs to keep the ADR-068 carve-out, or update the PR description and the schema-side CLAUDE.md policy line to drop the carve-out and explain why. Right now the diff and the policy contradict each other and a future reviewer can't tell which one is canonical. (The Rust API renames score_platform_for_rolescore_node_for_role etc. are independent of the prose question and look correct under either interpretation — Strike/Support are still role labels, and the function-name axis is substrate vocabulary.)

[WARNING] sed artifact in node.proto comment

peat-schema/proto/node.proto HumanMachinePair.node_ids:

-  // List of platform/node IDs
-  repeated string platform_ids = 2;
+  // List of node/node IDs
+  repeated string node_ids = 2;

The pre-rename comment intentionally used "platform/node" as a dual-vocabulary hint. After a verbatim sed platformnode it reads "node/node IDs". Should be "List of node IDs". Doc-only, won't change wire shape, but it's the kind of artifact that signals the rename pass wasn't reviewed comment-by-comment and is worth a follow-up grep before Phase 4/5 lands.

…ole carve-out

Addresses the Peat QA Review on 550cb22 (2 BLOCKER + 3 WARNING):

[BLOCKER] In-repo Android example left on the old FFI surface. The Rust JNI
symbols are renamed to Java_..._getNodesJni / publishNodeJni, but the Kotlin
externs/wrappers/callers and the TestRunner JSON were not, so the dual-peer
functional test would hit UnsatisfiedLinkError + a silent node_type drop.
Renamed getPlatformsJni/publishPlatformJni -> getNodesJni/publishNodeJni, the
getPlatformsJson()/publishPlatform() wrappers -> getNodesJson()/publishNode(),
all TestRunner call sites, and "platform_type" -> "node_type" in the JSON.

[BLOCKER, escalated from WARNING] peat-btle external API wrongly renamed. The
blanket platform->node sed rewrote peat_btle::platform::{android,apple,linux} ->
peat_btle::node::* and BleError::PlatformError -> NodeError in peat-ffi and the
peat-ble-test example. Published peat-btle 0.4.0 exposes only `platform` /
PlatformError (verified against the crates.io source); peat-ffi --features
bluetooth failed to compile. CI missed it because no job builds peat-ffi with
bluetooth on linux. Reverted to the real 0.4.0 API; peat-btle's own rename is a
separate substrate phase.

[WARNING] role.proto / models/role.rs carve-out. ADR-068, the PR description and
CLAUDE.md all preserve the "Strike/Support platform" role-label prose as a
vehicle-class term; the diff renamed it anyway. Restored it. Substrate renames
(platform_id->node_id, score_platform_for_role->score_node_for_role, etc.) kept.

[WARNING] node.proto sed artifact: "List of node/node IDs" -> "List of node IDs".

Verified: cargo check --workspace; peat-ffi --features bluetooth now compiles;
peat-protocol + dual_test_peer build; peat-ffi 57 lib tests pass; clippy clean;
both Kotlin files have zero platform refs.

Refs: #968

@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: a4a718a)

ADR-068 Phase 1 is a wholesale platform → node rename across the peat workspace (schema + protocol + transport + ffi + persistence + examples), plus a transitive peat-mesh bump from rc.31 → rc.35 for the Phase-3 HierarchyLevel::Node change. The proto field numbers are preserved (only names change), the storage collection name flips from "platforms" to "nodes", and the FFI exports (PlatformInfo/PlatformStatus/PLATFORMS/getPlatformsJni/publishPlatformJni/get_platforms/put_platform and the BleTranslator API peripheral_to_platform/platform_to_peripheral/platforms_collection) all rename. The PR description acknowledges the wire-break (pre-1.0 clean break) and the cross-phase lockstep dependencies (peat-node Phase 4 collection rename, peat-atak-plugin Phase 5 binding update).

Below are the findings the QA harness identified beyond what the latest fix commit (a4a718a) already addressed.

[BLOCKER] Variable shadowing in peat-ffi/examples/peat_cot_client.rs (publish_cells_and_nodes, update_node_positions) — example fails to compile

peat-ffi/examples/peat_cot_client.rs:380 defines fn publish_cells_and_nodes(node: &peat_ffi::PeatNode). Inside the loop body the rename produced:

peat-ffi/examples/peat_cot_client.rs:431:        let node = serde_json::json!({ ... });
peat-ffi/examples/peat_cot_client.rs:445:        let json = node.to_string();
peat-ffi/examples/peat_cot_client.rs:446:        match node.put_document("nodes", &node_id, &json) {

The local let node = serde_json::json!({...}) (line 431) shadows the node: &peat_ffi::PeatNode parameter. The subsequent node.put_document("nodes", &node_id, &json) (line 446) is then invoked on serde_json::Value, which has no put_document method — this is a compile-time error. The same shadowing pattern is repeated in update_node_positions at peat-ffi/examples/peat_cot_client.rs:454peat-ffi/examples/peat_cot_client.rs:463peat-ffi/examples/peat_cot_client.rs:480. Both examples are gated on required-features = ["sync"] and sync is the default feature.

The verification in the latest fix commit ("Verified: cargo check --workspace") does NOT compile examples — cargo check --workspace only covers the library/bin targets unless --all-targets is passed. The dual_test_peer example was checked explicitly because of the bluetooth feature flap, but peat_cot_client was not, and the shadowing slipped through.

This is the exact same bug class as the JNI-side fix already landed in peat-ffi/src/lib.rs:2011 (where let peat_node = unsafe { Arc::from_raw(...) } was introduced specifically to avoid the node: NodeInfo / node: Arc<PeatNode> collision). Apply the same shape here: rename the JSON local (e.g. let node_doc = serde_json::json!({...}) and node.put_document("nodes", &node_id, &node_doc.to_string())) or rename the parameter (peat_node: &peat_ffi::PeatNode).

Fix scope: 2 functions in 1 file. Re-verify with cargo build --workspace --all-targets --features sync (or at minimum cargo build -p peat-ffi --examples) before merging.

[BLOCKER] Ontology rename creates duplicate "node" concept with self-referential parent in peat-schema/src/ontology.rs

peat-schema/src/ontology.rs:149-158 now contains two consecutive add_concept calls that both register the id "node":

peat-schema/src/ontology.rs:149:    ont.add_concept(
peat-schema/src/ontology.rs:150:        Concept::new("node", "Node", ConceptCategory::Entity)
peat-schema/src/ontology.rs:151:            .with_description("A node in the Peat Protocol network (node or system)"),
peat-schema/src/ontology.rs:152:    );
peat-schema/src/ontology.rs:154:    ont.add_concept(
peat-schema/src/ontology.rs:155:        Concept::new("node", "Node", ConceptCategory::Entity)
peat-schema/src/ontology.rs:156:            .with_parent("node")
peat-schema/src/ontology.rs:157:            .with_description("A physical node (UAV, UGV, soldier system, etc.)"),
peat-schema/src/ontology.rs:158:    );

The first was the original root "node" concept (no parent); the second was previously Concept::new("platform", ...).with_parent("node") and got blanket-renamed. Ontology::add_concept at peat-schema/src/ontology.rs:103-105 uses HashMap::insert, so the second registration silently overwrites the first. The surviving "node" concept has parents = ["node"] — a self-loop.

is_subtype_of(concept_id, parent_id) at peat-schema/src/ontology.rs:113-127 recursively walks parents. For is_subtype_of("node", X) where X != "node" and X is not transitively reachable:

  • line 114 concept_id == parent_id → false ("node" != X)
  • line 118 get_concept("node") → Some(concept with parents=["node"])
  • line 119-122 loop body: is_subtype_of("node", X) → recurses with identical arguments

This recurses without termination → stack overflow. The shipped tests don't catch it (is_subtype_of("node", "node") short-circuits at the line-114 reflexive check; is_subtype_of("uav", "node") and is_subtype_of("cell", "node") walk different concepts), but any downstream caller asking is_subtype_of("node", "operator") or similar will crash.

Fix: delete the second Concept::new("node", ...).with_parent("node") block (peat-schema/src/ontology.rs:154-158). The substrate-level "node" concept and the physical-entity "node" concept are the same concept under ADR-068; the descendants (uav, ugv, soldier_system) already point at "node" correctly. Optionally merge the surviving description ("A node in the Peat Protocol network (physical UAV, UGV, soldier system, etc.)") if you want to preserve both intents.

While the file is open, two cosmetic sed artifacts in the same module are worth fixing in the same commit (not standalone findings, but adjacent):

  • peat-schema/src/ontology.rs:19/// Physical entities (nodes, nodes) (was "nodes, platforms")
  • peat-schema/src/ontology.rs:344 test comment — // node, node, uav, ugv, soldier_system (was "node, platform, ...")

[WARNING] peat-protocol/src/models/zone.rs doc-comment sed artifact

peat-protocol/src/models/zone.rs:4 reads //! - Nodes: Individual nodes with capabilities. The original prose was "Nodes: Individual platforms with capabilities". The blanket replace produced the redundant "nodes/nodes" phrasing, which is confusing to future readers since the three-tier list is Nodes / Cells / Zones. Either restore an alternative noun ("Nodes: Individual devices…") or rewrite the line to avoid the duplication.

Coverage / no-finding observations

  • FFI surface tests: every renamed public surface (get_nodes, put_node, NodeInfo, NodeStatus, getNodesJni, publishNodeJni, BleTranslator::nodes_collection, peripheral_to_node, node_to_peripheral) has its existing bidirectional E2E test renamed in lockstep — see peat-ffi/src/lib.rs mod node_tests (put_node_get_nodes_preserves_battery_and_heart, publish_json_inline_parser_extracts_battery_and_heart, serialize_nodes_get_json_round_trips_through_parser) and peat-ffi/tests/jni_wrapper_integration.rs scenario_publish_get_roundtrip + scenario_native_method_table_audit. Coverage is preserved at the same surface tier the PR-#841 / PR-#57 references describe.
  • FIPS posture: no crypto primitives touched; peat-mesh rc.31 → rc.35 bump is a pure schema-rename release.
  • Wire compatibility: proto field numbers preserved across all renames; only names changed. The collection-name change ("platforms""nodes") and FFI symbol renames are the wire-breaking surfaces, and the PR description explicitly calls out the lockstep peat-node Phase 4 and peat-atak-plugin Phase 5 coordination.
  • Role carve-out: peat-schema/proto/role.proto:18-19 correctly preserves CELL_ROLE_STRIKE / CELL_ROLE_SUPPORT with "Strike platform" / "Support platform" prose per ADR-068, as the a4a718a fix commit restored.

…tology self-loop

Addresses the Peat QA Review on a4a718a (2 BLOCKER + 1 WARNING):

[BLOCKER] peat-ffi/examples/peat_cot_client.rs failed to compile. In
publish_cells_and_nodes and update_node_positions the rename produced
`let node = serde_json::json!({...})`, shadowing the `node: &PeatNode`
parameter; the following `node.put_document(...)` then resolved against
serde_json::Value (no such method). Renamed the JSON local to `node_doc`
(same shape as the JNI-side peat_node fix). Missed earlier because
`cargo check --workspace` does not build examples — now verified with
`cargo build -p peat-ffi --examples` and `cargo check --workspace --all-targets`.

[BLOCKER] peat-schema/src/ontology.rs duplicate "node" concept with self-parent.
The rename turned `Concept::new("platform", ...).with_parent("node")` into a
second `Concept::new("node", ...).with_parent("node")`, which (HashMap::insert)
overwrote the root "node" concept and left it parenting itself — so
`is_subtype_of("node", X)` for any non-ancestor X recursed without termination
(stack overflow). Under ADR-068 the substrate participant and the physical
entity are one concept: collapsed to a single root "node" (no self-parent),
merged the description, and added a regression assertion that
`is_subtype_of("node", "capability")` terminates and returns false.

[WARNING] doc-comment sed artifacts: zone.rs "Individual nodes" -> "Individual
devices"; ontology.rs "Physical entities (nodes, nodes)" -> "(nodes)" and the
"node, node, uav..." test comment -> "node, uav...".

Verified: cargo check --workspace --all-targets; peat-ffi --examples build;
peat-schema 141 tests pass incl. the new is_subtype_of regression guard; fmt clean.

Refs: #968

@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: 8358944)

ADR-068 Phase 1 mechanical rename platformnode across the peat workspace. 77 files, +1213/-1275. Pre-1.0 clean wire break is acknowledged in the PR description with explicit Phase 4 (peat-node) and Phase 5 (peat-atak-plugin) coordination requirements. Schema field tag numbers preserved (e.g., DEVICE_TYPE_SENSOR_NODE = 7), so proto wire format remains tag-compatible; only field names and JSON keys are breaking. peat-mesh bump to 0.9.0-rc.35 aligns with ADR-068 Phase 3 already in main.

[WARNING] Over-zealous rename in OS/hardware-platform contexts contradicts the PR's stated carve-out

The PR description explicitly carves out two cases where "platform" refers to OS/hardware platform (role.proto "Strike/Support platform", ONNX "cross-platform"). The diff misses many functionally identical cases — comments and example code where "platform" means operating system / hardware platform, not a Peat substrate participant. Examples:

  • examples/peat-ble-test/src/main.rs: top-level doc section ## Platform Support (listing Linux/macOS/Linux-StubAdapter rows) renamed to ## Node Support; Platform is auto-detected (referring to OS detection for BLE adapter selection); Create transport using platform-specific adapter / with platform-specific adapter; Using StubAdapter - no real BLE hardware on this platformon this node.
  • examples/m5stack-core2-peat/src/nimble.rs:1035: Build advertising data with 128-bit UUID (standard across all platforms)across all nodes. The 128-bit UUID is BLE-protocol-cross-OS standardisation, unrelated to peat nodes.
  • peat-ffi/src/lib.rs (public DocumentCallback trait doc, lines ~526 and ~534): Outbound transport-frame callback for non-Android platforms (iOS via UniFFI)non-Android nodes; Implementations on non-Android platforms should expect any-thread invocationnon-Android nodes. This is in a public trait doc-comment that ships in cargo doc output.
  • peat-ffi/src/lib.rs:~700: block_on() alone doesn't guarantee this on all platforms (especially Android where the JNI thread may not have proper thread-local storage for the Tokio runtime handle)on all nodes (especially Android).
  • peat-ffi/src/lib.rs:~1566: On non-Android platforms, we can initialize BLE directlynon-Android nodes.
  • peat-ffi/src/lib.rs:~1655: BLE transport requested but not yet implemented for this platformfor this node.

Reading "non-Android nodes" or "BLE not implemented for this node" suggests a category of Peat nodes called Android, not a target OS, and will mislead future maintainers. The carve-out was the right policy; it just wasn't applied uniformly. Action: revert the rename in these locations (and audit the remaining node-in-OS-context sites — likely a grep for non-Android nodes, this node near #[cfg(target_os, all nodes in doc comments around BLE/JNI).

[IDIOM] Dead-weight test assertions left behind by the ontology concept merge

peat-schema/src/ontology.rs correctly merges the former platform concept into node (and the new self-loop regression test on line ~7076 is good — pins the round-2 fix). But the mechanical rename leaves two collapsed assertions in tests::test_build_ontology and tests::test_ontology_is_subtype:

  • test_build_ontology (line ~7042): assert!(ont.get_concept("node").is_some()); now appears twice consecutively (the second was previously the "platform" lookup).
  • test_ontology_is_subtype: assert!(ont.is_subtype_of("uav", "node")); appears twice (lines ~7053 and ~7061 — what were "uav is a platform" + "uav is a node transitively" collapsed to the same query); assert!(ont.is_subtype_of("node", "node")); (line ~7058, formerly "platform is a node") is also present at line ~7069 as the explicit reflexive check.

Doesn't fail anything, but every duplicated assert! is dead test surface. Collapse them and keep the explanatory comments that survive ("UAV is a node", "Cell is not a node", reflexive check, new no-self-loop regression).

Notes (not findings — recorded for the reviewer's context)

  • Surface coverage criterion: this is a rename, not a new public method/field. Existing codec round-trip tests in peat-ffi/src/lib.rs::tests::node_tests (formerly platform_tests) cover serialize_node_jsonparse_node_json, the JNI-publish helper parse_node_publish_json, the emit-side serialize_nodes_get_json, and storage-level put_nodeget_nodes. peat-ffi/tests/jni_wrapper_integration.rs::scenario_native_method_table_audit verifies the JNI registry has the new getNodesJni / publishNodeJni symbols (function-pointer match). The renamed JNI symbols are covered by the symbol-table audit plus the unchanged codec coverage at unit-test tier — sufficient for a name change.
  • Cargo.lock itertools 0.14.0 → 0.10.5 (under prost-build / prost-build-config) is a transitive change from the peat-mesh bump, not a direct dep change in this PR.
  • BLE translator (peat-protocol/src/sync/ble_translation.rs) renames are consistent across nodes_collection, peripheral_to_node, node_to_peripheral, and the postcard-encoded wire-shape comment correctly preserves the wire constraint distinction. Postcard struct layouts are untouched — BLE radio bytes are unaffected.
  • CoT <_peat_> extension XML attribute platform="..."node="..." (peat-protocol/src/cot/peat_extension.rs) is a wire-breaking change for any Peat-aware CoT consumer; falls under the documented Phase 5 coordination with peat-atak-plugin. Non-Peat-aware CoT clients ignore the extension.

…ead test asserts

Addresses the Peat QA Review on 8358944 (1 WARNING + 1 IDIOM):

[WARNING] Over-zealous rename in OS/hardware-platform contexts. The blanket
sed renamed "platform" -> "node" in doc-comments/examples where it means the
operating system / hardware platform, not a Peat substrate participant — the
same category as the documented cross-platform / OS carve-out, just applied
non-uniformly. Reverted to "platform":
  - peat-ffi/src/lib.rs: DocumentCallback trait docs "non-Android platforms"
    (x2), the block_on "on all platforms (especially Android)" comment, the
    "On non-Android platforms" BLE-init comment, and "not yet implemented for
    this platform".
  - examples/peat-ble-test/src/main.rs: "## Platform Support", "Platform is
    auto-detected", "platform-specific adapter" (x2), "no real BLE hardware on
    this platform".
  - examples/m5stack-core2-peat/src/nimble.rs: BLE 128-bit UUID "standard
    across all platforms" (cross-OS protocol constant).
Genuine Peat-node references (node id/endpoint, get_nodes, swarm membership,
"decode on this node") are unchanged.

[IDIOM] Collapsed dead duplicate assertions left by the ontology concept merge
in test_ontology_creation (duplicate get_concept("node")) and
test_ontology_is_subtype (duplicate is_subtype_of("uav","node") and
("node","node")), keeping the explanatory comments + the round-2 no-self-loop
regression assert.

Verified: cargo check --workspace --all-targets; peat-ffi --features bluetooth;
peat-schema 141 tests pass; fmt clean.

Refs: #968

@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: 1e47049)

ADR-068 Phase 1 is a wide but mostly mechanical platform → node rename across the workspace. Three prior QA rounds have already chased down the Android example, peat-btle external-API, role-label, ontology-self-loop, CoT example shadowing, and OS-platform carve-out classes of bug. The current diff is clean on the protocol/transport/security primitives surface (no crypto deviation, no Iroh/Automerge semantic change beyond the rc.35 lockstep bump that the existing pin already admits), the schema break is pre-1.0 as ADR-066 set the precedent, and the cross-repo lockstep requirements (peat-node Phase 4 collection name, peat-atak-plugin Phase 5 FFI binding rename) are correctly called out in the PR description. The role-label prose ("Strike platform" / "Support platform") and the OS/hardware-platform contexts are preserved per the ADR carve-out.

One round-3-class miss remains.

[BLOCKER] AAR canonical Kotlin binding still on the old Rust JNI symbol names

peat-ffi/android/src/main/kotlin/com/defenseunicorns/peat/PeatJni.kt (peat-ffi:215, peat-ffi:221) still declares:

@JvmStatic external fun getPlatformsJni(handle: Long): String
@JvmStatic external fun publishPlatformJni(handle: Long, platformJson: String): Boolean

But this PR renames the Rust JNI exports in peat-ffi/src/lib.rs (peat-ffi:7446, peat-ffi:7546) to Java_com_defenseunicorns_peat_PeatJni_getNodesJni / ..._publishNodeJni. The dynamic registration tables in JNI_OnLoad / nativeInit (peat-ffi:9299, 9311, 9944, 9969) likewise register the new names.

PeatJni.kt's own header docstring (peat-ffi:5–19) opens with exactly the failure mode this PR re-introduces:

Before 0.1.2, the AAR shipped only the native .so binaries — consumers had to hand-roll matching external fun declarations locally. That meant any rename or signature change in peat-ffi's Java_com_defenseunicorns_peat_PeatJni_* extern fns surfaced as an UnsatisfiedLinkError or RegisterNatives SIGABRT at instrumented-test runtime, not at PR-gate time on either side. … 0.1.2 (peat#886) ships this canonical declaration so all consumers import the same source of truth. Method signatures stay in lockstep with peat-ffi/src/lib.rs by definition — they live in the same release artifact.

Round-1 caught and fixed the identical class of drift in examples/android-ble-test/.../PeatJni.kt, but the actually-shipped AAR (peat-ffi/android/src/main/kotlin/.../PeatJni.kt) was missed because no CI job links the AAR against the renamed Rust .so. The downstream effect is what the header warns about: any consumer (peat-atak-plugin, mobile-app plugins, anyone using com.defenseunicorns:peat-ffi) calling PeatJni.getPlatformsJni(handle) or PeatJni.publishPlatformJni(...) hits UnsatisfiedLinkError at runtime — the symbol no longer exists on the Rust side — and PeatJni.getNodesJni(...) / publishNodeJni(...) won't compile because no Kotlin declaration exists.

The surface-tier audit test for this exact contract (peat-ffi/android/src/androidTest/kotlin/.../PeatJniSurfaceTest.kt, lines 369, 372, 456, 462–463) is also unmigrated — it still does PeatJni.getPlatformsJni(h) / publishPlatformJni(h, j), so even the AAR's own surface-coverage gate will SIGABRT instead of catching the drift. Per this repo's surface-tier coverage rule, the AAR's bidirectional E2E for these two methods is now broken (not merely missing), which is [BLOCKER] regardless of how comprehensive the JVM-side jni_wrapper_integration.rs and Rust-side node_tests coverage is.

Fix: rename in peat-ffi/android/src/main/kotlin/com/defenseunicorns/peat/PeatJni.kt:

  • getPlatformsJnigetNodesJni (declaration + docstring)
  • publishPlatformJni(handle, platformJson)publishNodeJni(handle, nodeJson) (declaration + docstring)
  • the inline publish<Type>Jni reference at peat-ffi:70 ("e.g. publishPlatformJni")

And in peat-ffi/android/src/androidTest/kotlin/com/defenseunicorns/peat/PeatJniSurfaceTest.kt:

  • peatJniRefGetPlatformspeatJniRefGetNodes (both the declaration at line 456 and the audit-list reference at line 369)
  • peatJniRefPublishPlatformpeatJniRefPublishNode (lines 372, 462–463)
  • The two "platforms" collection string literals at lines 259 and 315 (used as a JSON-collection name in forceStoreErrorForTesting_* tests) should be "nodes" to match the renamed collections::NODES = "nodes".

[IDIOM] Orphan legacy-alias comment in storage/mod.rs

peat-protocol/src/storage/mod.rs:134–136 retains:

// Legacy compatibility aliases (`Node*` retained as a pre-ADR-066 alias;
// earlier military-vocabulary aliases were removed alongside the
// Cell/Cohort/Federation/Coalition rename in peat#904).

The actual alias line (pub use node_store::NodeStore as PlatformStore;) was correctly deleted by this PR, but the explanatory block above was edited (Platform*Node*) instead of being deleted with it — it now describes nothing. Drop the three-line comment.

…er drift

Addresses the Peat QA Review on 1e47049 (1 BLOCKER + 1 IDIOM), plus a
proactively-found instance of the same binding-drift class.

[BLOCKER] The shipped AAR Kotlin binding still bound the old Rust JNI symbols.
peat-ffi/android/src/main/kotlin/.../PeatJni.kt declared `getPlatformsJni` /
`publishPlatformJni`, but this PR renamed the Rust exports to
Java_..._getNodesJni / publishNodeJni (and the JNI_OnLoad/nativeInit registration
tables register only the new names). Any consumer (peat-atak-plugin, mobile
plugins) would hit UnsatisfiedLinkError at runtime; the new names wouldn't
compile because no declaration exists. Round-1 fixed the *example* PeatJni.kt but
missed the canonical AAR one — no CI job links the AAR against the renamed .so.
Renamed the two externs (+ param `platformJson`->`nodeJson`), and migrated the
AAR surface-audit test (PeatJniSurfaceTest.kt): peatJniRefGetPlatforms->...Nodes,
peatJniRefPublishPlatform->...Node, the getPlatformsJni/publishPlatformJni calls,
and the two "platforms" collection literals -> "nodes" (matching collections::NODES).

[IDIOM] Dropped the orphan three-line "Legacy compatibility aliases" comment in
peat-protocol/src/storage/mod.rs — the `NodeStore as PlatformStore` alias it
described was already deleted by this PR, leaving the comment describing nothing.

[proactive] examples/kotlin-test (UniFFI-generated binding + Main.kt) still used
`sourcePlatform`; the Rust FFI field is now `source_node`. Aligned the generated
binding + example (sourcePlatform->sourceNode) so a regen/rebuild stays
consistent. Not CI-built, but it's the same drift class and the last "platform"
symbol in any .kt.

Repo-wide .kt is now free of platform symbols (bar the androidx.test.platform
framework import). Deliberately NOT touched: spec/proto/ + draft-peat-protocol-00.md
(the formal protocol-spec document tree — not compiled, separate from
peat-schema/proto/ which the build uses; a spec-revision-boundary follow-up).

Verified: cargo check --workspace --all-targets; peat-protocol builds; fmt clean.

Refs: #968

@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: b35b04b)

Phase 1 of ADR-068: mechanical platform → node rename across schema + in-repo consumers (proto fields, Rust types, UniFFI/JNI surface, storage collection key, CoT extension XML, ontology concepts). 79 files, almost entirely renames. Wire-breaking changes are explicitly tagged in the PR description as pre-1.0 clean breaks.

Surface-tier test coverage (per the bidirectional E2E rule): the renamed JNI extern functions are exercised end-to-end. peat-ffi/tests/jni_wrapper_integration.rs::scenario_publish_get_roundtrip round-trips through "nodes" collection via the real Java_..._publishNodeJni / Java_..._getNodesJni entry points, and scenario_native_method_table_audit pins the renamed function pointers in both JNI_OnLoad and nativeInit tables. peat-ffi/android/src/androidTest/.../PeatJniSurfaceTest.kt's reflection-based audit includes peatJniRefGetNodes and peatJniRefPublishNode. The Rust node_tests mod (formerly platform_tests) carries put_node_get_nodes_preserves_battery_and_heart, publish_json_inline_parser_extracts_battery_and_heart, and serialize_nodes_get_json_round_trips_through_parser — all exercise renamed codec helpers in both directions.

Cross-cutting checks:

  • peat-btle wire format unaffected: BleTranslator renames the collection key ("platforms""nodes") but the postcard BlePeripheral struct on the BLE radio is byte-identical. Wire-format break that the round-6 review caught for battery_percent is not reintroduced.
  • Automerge sync semantics / Iroh peer discovery unaffected — only doc-store collection-key strings change.
  • FIPS primitives unaffected (no crypto changes).
  • peat-lite / OTA / A/B unaffected (no peat-lite changes in this PR).
  • Ontology fix: the rename collapses the prior node → platform parent edge to a single root node concept. The PR correctly adds a is_subtype_of("node", "capability") regression test that would have caught the brief intermediate self-parent bug (infinite recursion).
  • peat-mesh dep bump (0.9.0-rc.310.9.0-rc.35) is consistent with the in-repo HierarchyLevel::Platform → HierarchyLevel::Node substitution in peat-persistence/src/adapters/beacon.rs; the workspace builds and 1,216 lib tests pass per the PR body.
  • Cross-phase coordination (peat-node Phase 4 collection rename, peat-atak-plugin Phase 5) is explicitly acknowledged in the PR description with the pre-1.0 clean-break policy.

[IDIOM]

  • peat-protocol/src/models/mod.rs:23-28 — the comment block "Legacy compatibility aliases for the Node* naming kept during the pre-ADR-066 migration window. …" remained after the pub use node::NodeConfig as PlatformConfig / pub use node::NodeState as PlatformState lines below it were deleted. The header now describes aliases that no longer exist. Delete the comment along with the empty section, matching the cleanup pattern used in peat-protocol/src/storage/mod.rs (which removed both the comment and the PlatformStore alias together). Non-blocking — pure dead documentation.

@kitplummer
kitplummer merged commit ec08c75 into main Jun 6, 2026
23 checks passed
@kitplummer
kitplummer deleted the feat/968-phase1-schema-node-rename branch June 6, 2026 21:07
kitplummer added a commit that referenced this pull request Jun 6, 2026
… schema/protocol) (#971)

Bumps the workspace 0.9.0-rc.22 → rc.23 and rolls the CHANGELOG [Unreleased]
section into the rc.23 notes. Moves the peat-protocol → peat-schema exact pin
to =0.9.0-rc.23 in lockstep.

Publishes #969 (ADR-068 Phase 1): peat-schema + peat-protocol converge on Node
as the base-unit term (platform_id→node_id, platform_type→node_type,
source/target_platform→node, collection "platforms"→"nodes"). Wire-breaking on
field names/JSON keys (proto field numbers preserved). Unblocks peat-node Phase 4,
which consumes published peat-schema/peat-protocol.

Refs: #968
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