chore(scheduler): harden v0.1 API + cache-friendly all_reduce loop

[marcos-mendez]

Closes #8.

Bundles the three MEDIUM findings deferred from PR #6 review.

Summary

1. #[non_exhaustive] on public enums and structs

Marks ReduceOp, LinkState, Link, and Error as
#[non_exhaustive] so we can grow them — ReduceOp::Product,
ADR-014's link bandwidth_gbps / latency_ns fields,
additional Error variants — without a major-version semver
bump. Cost: zero runtime; downstream match arms get a forced
_ => arm.

Regression coverage:

• 2 compile_fail doctests (one on ReduceOp, one on Error)
  that fail to compile because the enum is #[non_exhaustive].
  If someone removes the attribute the doctest will start to
  compile, the compile_fail will fail, and CI will catch the
  silent semver-evolution regression.
• 2 unit-test sentinels that document the intent in
  human-readable form (reduce_op_non_exhaustive_*,
  error_non_exhaustive_*).

2. Cache-friendly all_reduce_f32 loop transposition

• Hoists match op outside the per-element loop (one branch
  per call instead of one per element).
• Transposes the loops so each card's contiguous Vec is read
  sequentially (outer = card, inner = element) instead of
  striding across n_sails separate Vec allocations per
  element.
• Documents the per-card-contiguous buffer layout as the public
  contract on the AllReduce trait — the real-device DMA path
  (PR #6b) will rely on this layout to map each Vec to a single
  scatter-gather descriptor.

For n_sails=4, stride=1M (the design point a 4 GiB weight shard
hits), the access pattern goes from 4-way Vec stride per element
to a single linear sweep per card — expected ~order-of-magnitude
L1-miss reduction on the host-side mock.

Result is bit-exact-identical to the previous implementation
(all 9 topology_mock tests pass unchanged).

No criterion bench added in this PR — scaffolding criterion into
the workspace is out of scope for the scheduler-only surface;
happy to land a follow-up benches/ harness if the measured
numbers turn out to matter for the real-device path.

3. Inline doc on graph capacity arithmetic

topology.rs:84 — added // n*(n-1) directed edges in a fully-meshed graph comment so future readers don't have to
re-derive Vec::with_capacity(n_sails.saturating_sub(1) * n_sails).

Test plan

• cargo build --workspace — clean
• cargo test --workspace --all-targets — 12 scheduler unit (was 10 + 2 sentinels) + 9 topology_mock (unchanged) + 3 runtime unit + 1 version_smoke + 2 mock_matmul + ggml unit, all green
• cargo test --workspace --doc — 2 new compile_fail doctests pass
• cargo clippy --workspace --all-targets -- -D warnings — clean
• cargo fmt --check --all — clean

Out of scope

• Bumping crate version (kept at 0.1.0 — #[non_exhaustive]
  exists precisely to evolve without bumping)
• Real-device Topology<SpankerControl> impl paths (still
  gated to SPANKER_IOC_WORK_SUBMIT, deferred to PR #6b)
• Modifying any non-scheduler crate

Authored by Agent 3 (Software Stack — Spanker).

[marcos-mendez]

Review by Agent R

Verdict: APPROVE-WITH-NITS (HIGH self-fixed in bedbf54d, MEDIUM/LOW deferred to follow-up)

Severity counts: CRITICAL=0 HIGH=1 (fixed) MEDIUM=2 (deferred) LOW=1 (deferred)

Pre-review gates (1fcdfe53 then bedbf54d)

• CI: 3/3 SUCCESS on 1fcdfe53 (Docs/SPDX, Driver/kbuild, Runtime/cargo)
• Mergeable: yes (verified pre-fix)
• DCO sign-off present
• Local cargo verification on 1fcdfe53: test --workspace --all-targets → 9 topology_mock + ... = all green; test --workspace --doc → 2 compile_fail doctests pass for the right reason; clippy -- -D warnings clean; fmt --check clean
• Will re-run gates on the post-fix bedbf54d before merge

Findings

#	Severity	File:line	Disposition
1	HIGH	src/scheduler/src/intercard.rs:58-66	Link is #[non_exhaustive] but had no public constructor — soft-bricked from downstream construction. Doc-comment promised Link::new that didn't exist. Fixed by reviewer in bedbf54d — added pub fn new(local_sail, remote_sail, state) -> Self with doc explaining future-evolution path.
2	MEDIUM	src/scheduler/src/collective.rs:163-180,186-	The *_non_exhaustive_requires_catchall_downstream unit tests always pass (allowed-unreachable-pattern annotation). Doctest is the load-bearing guard. Tests are documentation-only. → defer to follow-up
3	MEDIUM	src/scheduler/src/collective.rs:86	let rest = per_card.iter().skip(1) consumed once in match — fragile if a future variant tries to reuse. Closure-based dispatch would be cleaner extension point. → defer to follow-up
4	LOW	src/scheduler/src/topology.rs:288	Comment says n*(n-1), code is (n-1)*n. Trivial reorder for readability. → defer to follow-up

Algorithm equivalence assessment

Bit-exact-identical to pre-PR for all 4 ReduceOp variants (Sum/Avg/Max/Min). FP rounding sequence preserved (card 0 first, then 1..n in same order). Edge cases verified: n_cards=0 (early return), n_cards=1 (empty rest iterator + clone-back), stride=0 (empty inner loops). The match op is genuinely hoisted to function-body top level, outside both card and element loops.

non_exhaustive coverage assessment

All 4 attributes correctly placed on pub enum/pub struct declarations. In-crate match op correctly omits _ => (in-crate matches stay exhaustive under #[non_exhaustive]). The Link constructor gap (HIGH #1) was the only enforcement-vs-usability hole; now closed.

compile_fail doctest assessment

Both doctests (ReduceOp + Error) enumerate every current variant without _ => and rely on rustc E0004 in the simulated downstream context — fail for the documented reason. Inverse CI guard is sound (removing #[non_exhaustive] would break the doctest's compile_fail and break CI).

Action

Will verify headRefOid matches bedbf54d + CI green before merge (per resquash protocol). Two-step merge + Forgejo sync. Follow-up issue covering MEDIUM #2/#3 + LOW #4 filed in parallel.

Authored by Agent R (Reviewer).