gistfile1.txt

Priority 1 — Real bug: registry first-insert race
AmalgafyRegistry::add_micro_joules has a TOCTOU window between get() and insert(). If two providers simultaneously see a PID for the first time, both will call insert(pid, AtomicU64::new(0)). crossbeam_skiplist::SkipMap::insert replaces on collision — it does not return "the entry that ends up living in the map" as the comment claims. Whichever thread's insert runs second overwrites the first, and the delta accumulated into the first AtomicU64 is silently dropped.
The comment is a plausible-sounding but incorrect assumption about the library's behavior. The fix is to add a second get after the insert, since the winning entry will be observable by both threads:
rustpub fn add_micro_joules(&self, pid: u32, delta_uj: u64) -> u64 {
    // Fast path — PID already tracked
    if let Some(entry) = self.table.get(&pid) {
        return saturating_add(entry.value(), delta_uj);
    }
    // Insert a zero slot, then immediately re-fetch so we accumulate onto
    // whichever entry won the concurrent-insert race, not necessarily ours.
    self.table.insert(pid, AtomicU64::new(0));
    let entry = self.table.get(&pid).expect("just inserted or raced in");
    saturating_add(entry.value(), delta_uj)
}
This race matters in practice whenever two providers (e.g. Linux RAPL + a GPU provider) call sync_registry concurrently for a PID that hasn't been seen before.

Priority 2 — Attribution promise vs. delivery
The README says MJAE will tell you Process 1024 "burned 287 W... because it saturated the 16-bit float units." The actual start_sampling_loop implementation does an equal split across active_pids(). That's fine as a baseline but it means the library doesn't currently deliver the per-process precision the documentation sells.
The data to do better already exists: NvmlProcessInfoV3 carries used_gpu_memory, gpu_instance_id, and compute_instance_id. The deterministic model in attribution.rs exists and is correct — it just isn't wired into start_sampling_loop. Consider either wiring it in or clearly marking the sampling loop docs as "equal-split baseline; use WindowsProvider::attribute_window for occupancy-weighted attribution."

Priority 3 — Apple Silicon idle calibration
AppleSiliconProvider::sample_power_state returns idle_power_uw: 0 unconditionally. The attribution math then treats 100% of active power as burst energy. There's no interface for operators to supply a calibrated idle baseline, which means macOS numbers are slightly inflated (especially at low utilization). A constructor parameter like new(hardware_sig, idle_power_uw: u64) — even just defaulting to 0 with a doc comment — would surface this as a deliberate choice rather than a hidden assumption.

Priority 4 — eBPF program gap
LinuxProvider ships the user-space side of the dma_fence:dma_fence_signaled hook but there's no .bpf.c, Aya eBPF object, or TracePoint attachment code in the library. Operators who pull in mjae on Linux need to build and attach the eBPF probe separately and drain the perf ring themselves before calling enqueue_attribution. This should be explicit in the README — the current architecture diagram implies it's self-contained.

Priority 5 — Signing key surface
AmalgafySigner::new(SigningKey::from_bytes(&secret)) is the only API. No guidance on TPM-backed key storage, OS keychain integration, or key rotation. A data-center daemon that leaks its Ed25519 private key loses seal integrity retroactively for every manifest ever published. This doesn't need to be solved in the library itself, but the README should include a "Key management" section that at minimum tells operators not to hardcode the key in an environment variable.

Minor items
manifest.rs (EnergyManifest) and signer.rs (AmalgafySeal) both implement a sign/verify pattern. The manifest type looks like an earlier layer that wasn't removed when the seal was added. Worth deciding which is the canonical public API and deprecating or removing the other.
The start_sampling_loop timeout gap is worth noting: if sample_power_state() hangs (e.g. IOKit deadlock, NVML timeout), the 100ms tick freezes indefinitely with no recovery. A tokio::time::timeout(Duration::from_millis(80), self.sample_power_state()) wrapper would keep the loop alive under telemetry pathologies.
LinuxProvider derives Clone, which means silently cloning a live provider creates a second independent state machine tracking the same hardware. This is unusual enough to warrant a #[doc = "Cloning creates an independent provider with its own pending buffer..."] note.