[feat][network] Add join/repair event payloads

[TheP2P (thep2p)]

Closes #84

[Copilot]

Pull request overview

Adds the join/repair message catalogue (Issue #84) to the core model and network event layer, aligning the codebase with the protocol spec in docs/protocol/concurrent-insert.md Section 2.

Changes:

• Added 8 new Event variants for join/repair messaging in src/network/mod.rs.
• Introduced src/core/model/join.rs with request/response payload structs for join + backpointer repair.
• Wired the new model module and re-exported the new payload types from src/core/mod.rs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/network/mod.rs	Adds new Event variants and imports corresponding payload types from core.
src/core/model/mod.rs	Registers the new join model module.
src/core/model/join.rs	Defines join/repair request/response payload structs per protocol spec.
src/core/mod.rs	Re-exports join payload types from core.
src/core/context/context_test.rs	Formatting-only adjustments in tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[TheP2P (thep2p)]

Partially correct — addressed in ebcb46c, which re-exports Direction, Identity, and Nonce from core, so all field types of the public payload structs are now externally nameable/constructible.

Two corrections for the record: (1) no private_interfaces/private_in_public warning fires — those lints key off the type's declared visibility, and both types are declared pub (just in unreachable modules, the allow-by-default unnameable_types situation); clippy with -D warnings was already clean. (2) The gap predates this PR: IdSearchReq (pub nonce: Nonce, pub direction: Direction) and the LookupTable trait's Direction parameters were already public before this change. The fix closes those pre-existing cases too.

[S T (staheri14)]

Looks good

[TheP2P (thep2p)]

Claude (@claude) review

[claude]

Claude finished TheP2P (@thep2p)'s task —— View job

---

Claude Code Review

• Read repo docs/protocol spec for guidelines
• Review src/core/model/join.rs
• Review src/network/mod.rs
• Review src/core/mod.rs re-exports
• Review src/core/model/mod.rs & test changes
• Post review feedback

---

Reviewed all 5 changed files against the protocol spec in docs/protocol/concurrent-insert.md §2. This is a clean, well-scoped, docs-only-plus-types PR that lands the message catalogue exactly as specified. Overall looks good — the earlier Copilot re-export gap is genuinely closed. A few observations below, none blocking.

Correctness vs. spec — matches ✅

• All 8 Event variants (GetMaxLevelOp … CheckNeighborOp) match the spec table (mod.rs:22-36), and every payload's field set/types match §2's table row-for-row (e.g. LinkReq{ nonce, candidate, side, level }, LinkRes{ nonce, side, level, linked }).
• Forwardable requests (LinkReq, BuddyReq, CheckNeighborReq) correctly carry full Identity, not just Identifier, per §2. Non-forwardable ones (MaxLevelReq, NeighborReq) correctly carry only origin: Identifier. 👍
• The receiver-owned side/direction semantics are documented on every field, consistent with §2's "never re-interpreted hop-to-hop" invariant. The module-level doc comment (join.rs:1-14) is excellent context.

Suggestions (non-blocking)

1. Missing derives — likely needed soon. Existing Nonce/Direction derive Hash, PartialEq, Eq, but the new payload structs derive only Debug, Copy, Clone. The spec (§3.0, §6.2) requires correlating replies by Nonce and comparing an incoming SetLinkOp/relink against the current entry (existing == claimant, existing == candidate). Consider adding PartialEq, Eq (and Hash where relevant) now to the payloads to avoid churn when the handlers land. Not required for a types-only PR, just flagging.

2. try_link/try_relink outcome types not in this PR. Fine given the PR scope (Closes [Network] Add: join/repair Event variants and payload structs #84 = message catalogue only), but note the spec's LinkOutcome/RelinkOutcome (§4.3) and the LookupTable trait additions are the natural next PR — the current join.rs gives no way to exercise these payloads yet, so there are intentionally no unit tests here. Worth confirming that's the agreed slice.

3. NeighborRes omits origin. NeighborReq carries origin: Identifier so the responder can reply directly, but NeighborRes (like MaxLevelRes) relies solely on nonce for correlation at the originator. That's consistent with the spec table and with how IdSearchRes works today (nonce-keyed), so this is correct — noting it only because the asymmetry can look surprising at first glance.

4. Re-export resolution (prior Copilot thread). Confirmed: core/mod.rs:13,15,27 now re-export Direction, Identity, and Nonce, so all public field types of the exported payload structs are externally nameable/constructible. The author's clarification that no private_in_public lint actually fired is accurate — the types are declared pub in pub(crate) modules (unnameable_types, allow-by-default). Fully addressed.

Nits

• context_test.rs changes are pure rustfmt line-wrapping — no behavioral change, correct to include.
• Nonce currently only has random() as a constructor and no accessor; once handlers need to key maps by it that's fine (it derives Hash/Eq), but external code re-exported via core can construct one only randomly — acceptable for now.

Verdict: Approve-equivalent. Clean, spec-faithful, correctly scoped. I could not run cargo build/clippy in this session (not in allowed tools), so I verified consistency by reading only; the code reads as compilable and the imports all resolve to existing paths.
• thep2p/84-join-repair-events

[TheP2P (thep2p)]

Re point 1: deferred — the spec's == checks compare Identity values (already PartialEq) and reply correlation keys on Nonce (already Hash/PartialEq/Eq); whole-payload equality isn't needed, and Debug, Copy, Clone matches the existing IdSearchReq/IdSearchRes convention. If it's ever needed, it's a non-breaking derive to add uniformly later.

Re point 2: confirmed — #84 is the message catalogue only; try_link/try_relink and handlers land in the follow-up issues. Points 3–4 need no action per your own analysis.