docs(thread_aware): add a thread-aware authoring guide

[Pato Sandaña (psandana)]

What & why

Adds a _documentation module to thread_aware with a task-oriented authoring guide, the piece the crate''s (otherwise reference-style) docs are missing. Follows the recoverable::_documentation pattern and renders on docs.rs.

Addresses 7552151 (publish the thread-aware authoring guide) and folds in 7722787 (the 3S / oxidizer-spawner migration experience).

Contents

• Why thread-awareness exists — the thread-per-core motivation and the "relocation is a performance cooperation, not a correctness guarantee" principle.
• Authoring a type — prefer the derive; #[thread_aware(skip)]; what the field-type bounds mean; hand-written impls.
• Choosing an implementation — derive vs. manual vs. [Unaware] vs. strategy [Arc], as a decision table.
• Anti-patterns (from the 3S migration) — Clone copies stored affinity instead of relocating; #[thread_aware(skip)] on a sole field is a silent no-op; don''t trust inherited markings; relocate the whole graph once at the boundary.
• Testing — the Tracker observation pattern (assert relocation reaches non-skipped fields and not skipped ones).
• Debugging & telemetry — *.thread_mismatch-style warnings, and why their absence doesn''t prove correctness.
• Validating correctness — what the compiler/derive/clippy check, and what only your tests can.

Notes

• All four code examples are doctested and pass under default and all-feature builds.
• The "what the generated bounds mean" section describes the field-type bound model implemented in fix(thread_aware_macros): express derive bounds on the relocated field type #740 (sibling story under the same feature); the two are best merged together.
• This is an initial draft — happy to adjust depth, structure, or voice to the team''s preference.

Refs AB#7552151, AB#7722787.

[Copilot]

🟡 Changes recommended

_documentation is currently exported unconditionally, which unintentionally commits it as part of the crate’s normal public API instead of keeping it doc/test-only like other crates’ _documentation modules.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a task-oriented authoring guide for thread_aware by introducing a dedicated _documentation module intended to render on docs.rs and complement the crate’s existing reference-style docs.

Changes:

• Exposes a new _documentation module from thread_aware to host longer-form guidance.
• Adds a comprehensive authoring guide covering implementation choices, common pitfalls, testing patterns, and debugging/telemetry.

File summaries

File	Description
crates/thread_aware/src/lib.rs	Exposes the new _documentation module from the crate root.
crates/thread_aware/src/_documentation/mod.rs	New documentation module containing the authoring guide and doctest examples.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (9794095) to head (550a1fe).

Additional details and impacted files

@@          Coverage Diff           @@
##            main     #742   +/-   ##
======================================
  Coverage   99.9%   100.0%           
======================================
  Files        634      634           
  Lines      84746    84746           
======================================
+ Hits       84744    84746    +2     
+ Misses         2        0    -2     

Flag	Coverage Δ
linux	93.3% <ø> (?)
linux-arm	93.3% <ø> (?)
windows	93.4% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[martintmk]

• this should go into thread_aware_core
• rename to just documentation (also could you apply the same rename across all our crates)
• protect with #[cfg(any(doc, test))] so this module doesn't become part of our public API

[Pato Sandaña (psandana)]

Thanks! I've gated it with #[cfg(any(doc, test))] (705d1a9) so the module is doc/test-only and out of the normal public API, matching the existing _documentation modules in recoverable and fetch.

On the other two points:

• Move to thread_aware_core: this guide is written around the thread_aware facade — #[derive(ThreadAware)], Unaware, the strategy-partitioned Arc, ThreadBuilder — none of which live in thread_aware_core. Placing it there would need thread_aware as a dev-dependency of core (which risks the cyclic-deps gate) and would break the intra-doc links to those facade types. Keeping it where those APIs live seems most useful to readers. If you'd instead like a separate, trait-contract-focused guide in core, I'm happy to split it — just let me know.
• Rename _documentation → documentation across all crates: glad to, but since every crate currently uses the _documentation name it's a repo-wide convention change; I'd rather land it as its own sweep so it isn't tangled with this guide. I'll open a follow-up unless you'd prefer it here.

[Pato Sandaña (psandana)]

Gated with #[cfg(any(doc, test))] (705d1a9). Opened AB#7857350 for the repo-wide _documentation -> documentation rename so it can land as its own sweep. On the core move: this guide leans on the thread_aware facade - the derive, Unaware, strategy Arc, Relocator - none of which live in thread_aware_core, so moving it there breaks those intra-doc links (and would need thread_aware as a dev-dependency of core, risking the cyclic-deps gate). Happy to instead split out a separate, trait-contract-focused guide for core if you'd prefer.

[Pato Sandaña (psandana)]

Update on the "move to thread_aware_core" point: I've switched the guide's references to the facade types (the ThreadAware derive, Unaware, Arc, Relocator) to docs.rs links (9760e99), matching the pattern thread_aware_core already uses for these same types ([arc]: https://docs.rs/thread_aware/latest/thread_aware/struct.Arc.html, [derive]: .../derive.ThreadAware.html).

That removes the blocker I raised earlier: the guide no longer needs any of those types in scope, so it could move to thread_aware_core without a dev-dependency on thread_aware (and the dependency cycle that would create). It also fixes the feature-gated-link breakage — Arc is std-gated and Relocator is test-utils-gated, so intra-doc links to them broke under --no-default-features; docs.rs links resolve in every config. The core types it still links by intra-doc (ThreadAware, Thread, relocate) resolve from either crate.

Happy to move the module to thread_aware_core in this PR if you'd prefer, or land it here and move it in the rename sweep (AB#7857350). Let me know which you'd like.

[Copilot]

🔵 Needs a closer look

The new _documentation module export and its links should be aligned with the workspace’s doc/test-only _documentation pattern and avoid broken intra-doc links under default feature docs.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:230

• This doc comment links to Relocator via an intra-doc link (crate::Relocator), but Relocator is behind the test-utils feature in this crate. When building docs with default features (without test-utils), this becomes a broken intra-doc link.

Consider referring to it as code (or conditionally documenting it) so the guide doesn’t produce broken links in default doc builds.

crates/thread_aware/src/lib.rs:181

• _documentation modules in this workspace are typically doc/test-only so they render on docs.rs and in doctests without becoming part of the crate’s normal public API. Other crates gate the public _documentation module with #[cfg(any(doc, test))] (e.g., crates/recoverable/src/lib.rs:81-82, crates/fetch/src/lib.rs:860-862), but thread_aware currently exports it unconditionally here. Also, the (_documentation) markdown link is a relative URL and will resolve incorrectly when this doc comment is rendered on the module’s own page.

Consider gating the module and dropping the relative self-link (or converting it to an intra-doc link) to match the established pattern.

/// A guide to authoring thread-aware types: how to implement, test, and debug them, and the
/// anti-patterns to avoid. See [the guide](_documentation).
pub mod _documentation;

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new guide contains a couple of documentation correctness issues (notably conflicting bounds semantics and missing std feature context for Arc) that should be reconciled before publishing.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:90

• This section claims the derive bounds the relocated field type (e.g. emitting where Vec<T>: ThreadAware), but the derive macro's own docs in crates/thread_aware/src/lib.rs currently describe per-parameter T: ThreadAware bounds (see "# Generic Bounds" around lib.rs:211-217). To avoid contradicting the crate’s existing reference docs, consider rephrasing this section to describe bounds in terms of the derive’s traversal and link to the authoritative rules.
  crates/thread_aware/src/_documentation/mod.rs:122
• Arc/PerThread/PerProcess are only exported when the std feature is enabled (see crates/thread_aware/src/lib.rs:263-266), but the decision table currently presents them without that constraint. Adding an explicit "(std feature)" note would make the guidance accurate for no_std users reading this guide.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[martintmk]

could we link to actual macro (in external crate)

[Pato Sandaña (psandana)]

Done in 53ca92f - now links to macro@crate::ThreadAware.

[martintmk]

this should just link to main lib.rs docs, too much duplication

[Pato Sandaña (psandana)]

Trimmed to a pointer at the crate-level Theory of Operation (53ca92f); dropped the duplication.

[martintmk]

this might be too much detail, I myself had trouble grasping what is this traying to say

[Pato Sandaña (psandana)]

Agreed - cut it down to a short pointer at the derive's Generic Bounds reference (53ca92f).

[martintmk]

I am missing thread_aware::Arc guide here, when to implement this. (when we want to maintain separated PerThread instances)

[Pato Sandaña (psandana)]

Added a "Per-worker state with Arc" section (53ca92f): when to reach for Arc<T, PerThread> to keep separate per-worker instances, vs PerProcess / PerNumaNode.

[martintmk]

check thread_aware::Relocator (under "test-util") that could be used for relocation testing. Also too much detail too, make it more concise.

[Pato Sandaña (psandana)]

Made Testing more concise and now lead with the test-utils Relocator helper for driving relocations (53ca92f).

[martintmk]

we don't have any answer for this right now, I would just omit this section

[Pato Sandaña (psandana)]

Removed the Debugging and telemetry section (53ca92f).

[Copilot]

🟡 Changes recommended

The new docs introduce several feature-gated / derive-macro intra-doc links that will be broken or misleading in non-all-features doc builds and should be adjusted before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

crates/thread_aware/src/lib.rs:180

• The module doc comment uses a Markdown URL link ([the guide](_documentation)), which is easy to interpret as a relative URL rather than a rustdoc intra-doc link. Since this comment is on the _documentation module itself, the extra link is also redundant; removing it avoids brittle/ambiguous linking.

crates/thread_aware/src/_documentation/mod.rs:79

• This section header link targets the derive macro docs; it should use the derive@ disambiguator (not macro@) to match how derive macros are linked elsewhere in the repo and to avoid ambiguity with non-derive macros.

//! [Generic Bounds](macro@crate::ThreadAware#generic-bounds) reference has the rules.

crates/thread_aware/src/_documentation/mod.rs:121

• These table rows link to crate::Arc, which is std-feature gated. In non-std doc builds this becomes a broken intra-doc link and the table reads as if Arc were always available. Consider marking these rows as std-only and using inline code instead of intra-doc links.

//! | Shared state that should differ per worker | [`Arc<T, PerThread>`](crate::Arc) | Materializes a separate `T` per destination. |
//! | Shared state that is the same everywhere | [`Arc<T, PerProcess>`](crate::Arc) | Behaves as a vanilla `Arc`. |

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The new guide contains feature-gated intra-doc usage (crate::Arc, #[derive(ThreadAware)]) that should be shimmed/worded to remain correct across the crate’s supported feature sets.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:46

• This example uses #[derive(ThreadAware)], but the derive macro is feature-gated (pub use ::thread_aware_macros::ThreadAware is behind feature = "derive"). Wrapping the doctest body in a hidden #[cfg(feature = "derive")] shim keeps the guide’s examples copy/paste-able across feature sets (and matches the crate-level docs’ pattern).

This issue also appears in the following locations of the same file:

• line 58
• line 197
  crates/thread_aware/src/_documentation/mod.rs:72

• Arc is only exported when the std feature is enabled (see lib.rs’s #[cfg(any(test, feature = "std"))] pub use ... Arc). Linking to it here makes this paragraph only correct in std builds; prefer plain code and explicitly mention the std feature, as you do later in the guide.

crates/thread_aware/src/_documentation/mod.rs:62

• This doctest also depends on the derive feature for #[derive(ThreadAware)]. Consider gating the doctest body with a hidden #[cfg(feature = "derive")] block so the documentation remains valid when derive is disabled.

//! ```rust
//! use thread_aware::ThreadAware;
//!
//! #[derive(ThreadAware)]
//! struct Request {

crates/thread_aware/src/_documentation/mod.rs:201

• This doctest uses #[derive(ThreadAware)] for UnderTest, which is unavailable when the derive feature is off. Gating just the derived portion with a hidden #[cfg(feature = "derive")] shim keeps the rest of the example (the Tracker observation pattern) visible while making the doctest resilient to different feature sets.

//! #[derive(ThreadAware)]
//! struct UnderTest {
//!     tracked: Tracker,
//!     #[thread_aware(skip)]
//!     skipped: Tracker,

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Pato Sandaña (psandana)]

Thanks — addressed in c90d9f4:

• crate::Arc link (line 72): good catch, I missed one. That last [Arc](crate::Arc) in "Skipping a field" is now a plain code span naming the std feature, consistent with the rest of the guide. Verified the module contributes zero broken-intra-doc-link warnings in the --no-default-features doc build.

• Gating #[derive(ThreadAware)] doctests behind #[cfg(feature = "derive")]: I looked into this and don't think it applies here. docs/feature-gated-doctests.md scopes that rule to non-default features, and CI runs doctests with default features and --all-features — derive is in default = ["std", "derive"], so it's present in both legs. The crate's own convention matches: the ThreadAware derive example in lib.rs (# Example) is not derive-shimmed, and the std-gated example there wraps only the std-dependent body (Arc/PerThread) in # #[cfg(feature = "std")], using #[derive(ThreadAware)] freely inside. None of this guide's doctests use std-gated items (they import only Thread/ThreadAware, both unconditional), so no std shim is needed either. Adding derive shims would diverge from the crate's existing derive examples. Happy to add them if you'd prefer the belt-and-braces version.

[Copilot]

🔵 Needs a closer look

Several documented APIs and behaviors do not match the current implementation, and some doctests need feature-gated shims.

Review details

Suppressed comments (7)

Previously missed (4) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:82

• At this revision, the derive still adds ThreadAware directly to each reached generic parameter (crates/thread_aware_macros_impl/src/lib.rs:121-133), and the public derive reference describes that same parameter-bound model (crates/thread_aware/src/lib.rs:168-174). The field-type-bound behavior described here is therefore not what this checkout implements, so the guide will be stale if it lands before #740; either land the implementation change with this documentation or describe the current behavior.
  crates/thread_aware/src/_documentation/mod.rs:100
• Vec::new() does not by itself place the next allocation on the destination NUMA node; the core trait documentation notes that Vec cannot choose a node and that dropping only gives an allocator a chance to place future memory (crates/thread_aware_core/src/thread_aware.rs:115-118). This example currently presents NUMA-local reallocation as a guarantee even though it ignores destination and stores no NUMA state. Please describe this as discarding a cache, or show the allocator/state needed for NUMA placement.
  crates/thread_aware/src/_documentation/mod.rs:125
• thread_aware does not export an Arc (see crates/thread_aware/src/lib.rs:218-229); the strategy pointer is performables::arc::Arc (crates/performables/src/arc/mod.rs:150-165). The documented PerNumaNode name is also not defined—the strategy is PerNuma (crates/performables/src/arc/mod.rs:30-36)—and that crate has no std feature (crates/performables/Cargo.toml:29-40). As written, this sends readers to a nonexistent thread_aware::Arc API and strategy. Please update this section and the table to name performables::arc::{Arc, PerThread, PerProcess, PerNuma} (or remove the section if this guide is not meant to refer to that crate).
  crates/thread_aware/src/_documentation/mod.rs:137
• The PR description promises a Debugging & telemetry section covering *.thread_mismatch-style warnings and why their absence is not proof of correctness, but this module has no such section or example; it only says that most failures produce no warning. Please add the promised guidance or remove it from the description so the published guide's scope matches the PR.

crates/thread_aware/src/_documentation/mod.rs:144

• The trait is open-ended and does not require affinity to be stored in a field updated only by relocate; the core contract explicitly allows implementations to share or detach state (crates/thread_aware_core/src/thread_aware.rs:33-45). Thus Clone is not universally guaranteed to copy affinity verbatim. Please qualify this section to types whose Clone preserves the state that relocate updates.

//! This is the one to internalize first. A thread-aware type stores its affinity in a field that
//! only [`relocate`](crate::ThreadAware::relocate) mutates. **`Clone` copies that stored affinity
//! verbatim.** Cloning a value that was built on worker A and using the clone on worker B does not
//! move it to B - it is still bound to A, quietly, until something calls `relocate`.

crates/thread_aware/src/_documentation/mod.rs:36

• These examples (and the Request/UnderTest blocks below) invoke #[derive(ThreadAware)] unconditionally, but this macro is re-exported only under cfg(any(test, feature = "derive")) (crates/thread_aware/src/lib.rs:218-219). Since _documentation itself is included under cfg(doc) (crates/thread_aware/src/lib.rs:138-139), the guide needs the hidden feature-off shims required by docs/feature-gated-doctests.md so the examples also compile when derive is disabled.

//! #[derive(ThreadAware)]

crates/thread_aware/src/_documentation/mod.rs:31

• The derive does not forward relocation to fields marked #[thread_aware(skip)]; it moves those fields without invoking relocate (as described in the next section and implemented by the derive). Saying it forwards to “every field” here is misleading—please qualify this as every non-skipped field.

//! [the derive macro](https://docs.rs/thread_aware/latest/thread_aware/derive.ThreadAware.html). It
//! generates a [`relocate`](crate::ThreadAware::relocate) that forwards the notification to every
//! field, which is exactly what a compound type owes its parts:

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Address the outstanding doctest, NUMA wording, test invocation, and advertised telemetry-content issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

crates/thread_aware/src/_documentation/mod.rs:101

• Vec::new() does not itself place memory on the destination NUMA node. The core trait documentation explicitly notes that Vec cannot choose a node and that placement comes from the allocator (crates/thread_aware_core/src/thread_aware.rs:115-118), so this example overstates what the shown implementation guarantees. Please describe this as giving a NUMA-aware allocator an opportunity to choose a destination-local allocation instead.

//!         // The scratch buffer belonged to the previous worker; drop it so the next use
//!         // re-allocates in the destination's NUMA node instead of reaching across.
//!         self.buffer = Vec::new();

crates/thread_aware/src/_documentation/mod.rs:144

• This presents copied affinity as a property of every thread-aware type, but the ThreadAware contract neither requires Clone nor requires an affinity field; it explicitly allows implementations to retain, replace, or detach optimization state (crates/thread_aware_core/src/thread_aware.rs:38-45). Qualify this anti-pattern to types whose Clone implementation copies affinity-bearing state, so readers do not infer a guarantee that the trait does not make.

//! This is the one to internalize first. A thread-aware type stores its affinity in a field that
//! only [`relocate`](crate::ThreadAware::relocate) mutates. **`Clone` copies that stored affinity
//! verbatim.** Cloning a value that was built on worker A and using the clone on worker B does not
//! move it to B - it is still bound to A, quietly, until something calls `relocate`.

crates/thread_aware/src/_documentation/mod.rs:113

• Arc is not part of thread_aware: the strategy-based implementation is performables::arc with PerThread, PerNuma, and PerProcess (crates/performables/src/arc/mod.rs:26-36), and PerNumaNode is not a repository type. The current wording also says the destination value is created on first use, but this implementation materializes it during relocate (crates/performables/src/arc/mod.rs:418-475). Please point readers at the correct crate and names and describe when materialization occurs; the decision table below needs the same correction.

//! pool you do not want contended across cores - wrap it in the strategy-partitioned `Arc<T, S>`
//! ([`thread_aware::Arc`](https://docs.rs/thread_aware/latest/thread_aware/struct.Arc.html), with
//! the crate's `std` feature). With the `PerThread` strategy,
//! relocation materializes a separate `T` for the destination worker (lazily, on first use there),
//! so the sharing is per-worker instead of process-wide. Use `PerProcess`, which behaves as a

crates/thread_aware/src/_documentation/mod.rs:82

• This guide describes the field-type-bound derive model, but that implementation is not in the current base: thread_aware_macros_impl/src/lib.rs:121-134 still adds ThreadAware to each reached generic parameter, and the linked #740 is still open/unmerged. Merging this documentation change independently would make the public guide disagree with the derive users receive; please coordinate the merge/dependency or keep this section aligned with the current implementation until #740 lands.

//! You rarely need to reason about this: the derive adds exactly the `ThreadAware` bounds its
//! generated body needs and no more, so a correct type "just derives". When it matters - a generic
//! wrapper, or a marker field that should stay bound-free - the derive's
//! [Generic Bounds](https://docs.rs/thread_aware/latest/thread_aware/derive.ThreadAware.html#generic-bounds)
//! reference has the rules.

crates/thread_aware/src/_documentation/mod.rs:213

• Although this section is presented as a test, the doctest only defines assert_reaches_the_right_fields; it never calls the function with from and to, so neither assertion is executed. Add an invocation with synthetic coordinates (or explicitly label this as a helper snippet) so the guide actually demonstrates a test that can fail when forwarding or skip behavior regresses.

//! fn assert_reaches_the_right_fields(from: Option<&Thread>, to: &Thread) {
//!     let mut value = UnderTest {
//!         tracked: Tracker::default(),
//!         skipped: Tracker::default(),
//!     };

crates/thread_aware/src/_documentation/mod.rs:235

• The PR description lists a dedicated “Debugging & telemetry” section covering *.thread_mismatch-style warnings and why their absence is not proof of correctness, but this module goes directly from # Testing to # Validating correctness and contains none of that material. Please add the promised debugging guidance or remove it from the advertised contents so the published guide matches the scope described by the PR.

//! # Validating correctness
//!
//! What the toolchain checks for you, and what it cannot:
//!
//! * **The compiler** enforces the `ThreadAware: Send` supertrait and, through the derive's

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Pato Sandaña (psandana)]

Fixed in 180583e. Rather than shim each doctest, I gated the whole module on the derive feature: #[cfg(all(any(doc, test), feature = "derive"))]. Every example here is built around #[derive(ThreadAware)], so the guide is present exactly when the derive it documents is available (default and all-feature builds) and simply absent otherwise — which is what docs/feature-gated-doctests.md requires. Verified the guide doctests run under default features and are absent (no failure) under --no-default-features.

[Copilot]

🔵 Needs a closer look

The guide contains API and behavior inaccuracies and lacks promised telemetry guidance.

Review details

Suppressed comments (8)

Previously missed (2) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:114

• The public strategy is named performables::arc::PerNuma (crates/performables/src/arc/mod.rs:34-36), not PerNumaNode. As written, this identifier cannot be used with the current API.
  crates/thread_aware/src/_documentation/mod.rs:240
• These bullets describe the field-type-bound behavior from #740, but the derive currently checked in still adds ThreadAware bounds to each reached generic parameter (crates/thread_aware_macros_impl/src/lib.rs:121-134) and has no field-type predicate/deduplication logic. Because #740 is a separate unmerged PR, this guide is inaccurate if the docs change lands independently; please make that dependency/atomic merge explicit or keep the guide aligned with the current derive.

crates/thread_aware/src/_documentation/mod.rs:144

• ThreadAware only requires Send and relocate (crates/thread_aware_core/src/thread_aware.rs:146-161); it does not prescribe storage or Clone semantics. This paragraph overstates a migration-specific failure mode as a property of every thread-aware type and can mislead authors whose Clone implementation rebinds or recomputes state. Qualify it to clones that copy affinity-bearing state without rebinding.

//! This is the one to internalize first. A thread-aware type stores its affinity in a field that
//! only [`relocate`](crate::ThreadAware::relocate) mutates. **`Clone` copies that stored affinity
//! verbatim.** Cloning a value that was built on worker A and using the clone on worker B does not
//! move it to B - it is still bound to A, quietly, until something calls `relocate`.

crates/thread_aware/src/_documentation/mod.rs:101

• Because this example ignores _destination and only replaces the buffer with Vec::new(), it does not select or guarantee a destination-NUMA allocation; whether a later allocation is local is allocator/runtime-dependent. The crate docs frame reallocation in a new NUMA region as a possible optimization (crates/thread_aware/src/lib.rs:27-37), so this comment should not present it as the result of this implementation.

//!         // The scratch buffer belonged to the previous worker; drop it so the next use
//!         // re-allocates in the destination's NUMA node instead of reaching across.
//!         self.buffer = Vec::new();

crates/thread_aware/src/_documentation/mod.rs:111

• This presents strategy-partitioned Arc as a thread_aware facility, but thread_aware does not export Arc or these strategies (crates/thread_aware/src/lib.rs:227-233); they are provided by performables::arc (crates/performables/src/arc/mod.rs:21-36,150-170). Readers following this guide cannot obtain the shown type from the documented crate, so name the companion crate/dependency or remove this section.

//! pool you do not want contended across cores - wrap it in the strategy-partitioned `Arc<T, S>`
//! ([`thread_aware::Arc`](https://docs.rs/thread_aware/latest/thread_aware/struct.Arc.html), with
//! the crate's `std` feature). With the `PerThread` strategy,

crates/thread_aware/src/_documentation/mod.rs:112

• For the affinity-backed Arc, relocation eagerly invokes factory.materialize before returning (crates/performables/src/arc/mod.rs:465-468); it is not deferred until the value is first used or dereferenced on the destination worker. Please describe this as materialization on the first relocation targeting that worker so callers do not underestimate relocation cost.

//! relocation materializes a separate `T` for the destination worker (lazily, on first use there),

crates/thread_aware/src/_documentation/mod.rs:101

• This manual implementation drops the buffer on every relocation and ignores the source/destination coordinates. ThreadAware permits repeated calls, including when the destination is unchanged, and the core trait guidance recommends returning early and notes that Vec cannot select a NUMA node (crates/thread_aware_core/src/thread_aware.rs:70-73,108-120); as written, repeated calls needlessly discard reusable storage and the comment overstates NUMA placement.

//!     fn relocate(&mut self, _source: Option<&Thread>, _destination: &Thread) {
//!         // The scratch buffer belonged to the previous worker; drop it so the next use
//!         // re-allocates in the destination's NUMA node instead of reaching across.
//!         self.buffer = Vec::new();

crates/thread_aware/src/_documentation/mod.rs:233

• The PR description promises a “Debugging & telemetry” section covering *.thread_mismatch-style warnings, but this module goes directly from # Testing to # Validating correctness and never discusses that topic. Add the promised guidance or remove it from the stated contents so the documentation scope matches the change.

//! # Validating correctness
//!
//! What the toolchain checks for you, and what it cannot:

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The guide contains documentation inaccuracies and an unexecuted observation example that should be corrected.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

crates/thread_aware/src/_documentation/mod.rs:114

• PerNumaNode is not the strategy exposed by this repository: performables/src/arc/mod.rs:34-36 defines PerNuma, and its implementation is at performables/src/arc/mod.rs:746-762. The guide therefore teaches an identifier users cannot use; please use the actual PerNuma name here.

//! vanilla `Arc`, when one shared instance is what you want, and `PerNumaNode` for one instance per

crates/thread_aware/src/_documentation/mod.rs:144

• The trait contract does not prescribe Clone semantics (thread_aware_core/src/thread_aware.rs:33-36 explicitly says there are no exact semantic prescriptions), so a custom Clone implementation may reset or rebind affinity rather than copy it. Make this warning conditional on the type's Clone implementation copying its affinity-bearing state; otherwise the guide overgeneralizes a common pitfall into a trait rule.

//! This is the one to internalize first. A thread-aware type stores its affinity in a field that
//! only [`relocate`](crate::ThreadAware::relocate) mutates. **`Clone` copies that stored affinity
//! verbatim.** Cloning a value that was built on worker A and using the clone on worker B does not
//! move it to B - it is still bound to A, quietly, until something calls `relocate`.

crates/thread_aware/src/_documentation/mod.rs:209

• This doctest only defines assert_reaches_the_right_fields; nothing in the block calls it, so rustdoc type-checks the assertions but never executes them. Because this section presents the snippet as the observation pattern, make the example invoke the helper (or turn it into an executable test with concrete Thread coordinates).

//! fn assert_reaches_the_right_fields(from: Option<&Thread>, to: &Thread) {

crates/thread_aware/src/_documentation/mod.rs:112

• PerThread does not defer construction until first use: relocate_affinity invokes factory.materialize while processing Arc::relocate (crates/performables/src/arc/mod.rs:418-477), and the factory constructs the value immediately (crates/performables/src/arc/factory.rs:105-109). Calling this lazy gives authors the wrong relocation-cost and constructor-timing model; describe it as being created when relocation first encounters a destination partition.

//! relocation materializes a separate `T` for the destination worker (lazily, on first use there),

crates/thread_aware/src/_documentation/mod.rs:233

• The PR description promises a Debugging & telemetry section covering *.thread_mismatch warnings and why their absence is not proof of correctness, but this module goes straight from # Testing to # Validating correctness and contains no such guidance. Please add that promised section with the actual telemetry contract, or remove it from the stated contents.

//! # Validating correctness
//!
//! What the toolchain checks for you, and what it cannot:

crates/thread_aware/src/lib.rs:141

• The module is gated on derive, but the guide contains a hand-written implementation example at lines 84-104 that does not use the derive macro. The explanation that every example is built around #[derive(ThreadAware)] is therefore inaccurate; qualify it as referring to the derive-based doctests instead of implying the manual section depends on the feature.

/// Gated on `derive` because every example is built around `#[derive(ThreadAware)]`, which is only
/// available with that feature; this keeps the guide's doctests valid in a build without it (they
/// are simply absent) per `docs/feature-gated-doctests.md`.

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Documentation inaccuracies and a missing promised telemetry section remain unresolved.

Review details

Suppressed comments (5)

Previously missed (3) — in code that hasn't changed since the last review.

crates/thread_aware/src/_documentation/mod.rs:82

• This section documents the field-type-bound model from #740, but the derive present in this checkout still adds ThreadAware to each reached generic parameter in crates/thread_aware_macros_impl/src/lib.rs:121-133; the public derive reference likewise describes parameter bounds at crates/thread_aware/src/lib.rs:173-176. Since #740 is currently unmerged, publishing this guide would claim support for cases the implementation does not provide. Merge the implementation and guide together, or describe the current derive behavior instead.
  crates/thread_aware/src/_documentation/mod.rs:115
• Arc is not a type in thread_aware: the strategy implementation lives in the separate performables::arc module, and its NUMA strategy is named PerNuma, not PerNumaNode (crates/performables/src/arc/mod.rs:26-36; thread_aware has no performables dependency). As written, this section sends users to APIs that do not exist; point it at performables::arc and use PerNuma (the table below repeats the Arc shorthand).
  crates/thread_aware/src/_documentation/mod.rs:233
• The PR description promises a Debugging & telemetry section covering *.thread_mismatch-style warnings and why their absence does not prove correctness, but this module contains no such section or guidance. Add that promised material or remove it from the stated contents so the authoring guide matches the PR scope.

crates/thread_aware/src/_documentation/mod.rs:100

• Vec does not choose a NUMA node: dropping it only gives the application's allocator an opportunity to place a later allocation near the destination (crates/thread_aware_core/src/thread_aware.rs:115-120). Please qualify this example and make explicit that replacing the field is safe only for disposable scratch state, rather than promising reallocation in the destination node.

//!         // The scratch buffer belonged to the previous worker; drop it so the next use
//!         // re-allocates in the destination's NUMA node instead of reaching across.

crates/thread_aware/src/_documentation/mod.rs:144

• These sentences make copying affinity look like a ThreadAware/Clone guarantee, but the trait places no constraint on Clone; an implementation may rebind state or not store affinity at all. Scope this warning to derived/fieldwise Clone (or types whose clone preserves the stored affinity) so the guide does not reject valid custom clone implementations.

//! This is the one to internalize first. A thread-aware type stores its affinity in a field that
//! only [`relocate`](crate::ThreadAware::relocate) mutates. **`Clone` copies that stored affinity
//! verbatim.** Cloning a value that was built on worker A and using the clone on worker B does not
//! move it to B - it is still bound to A, quietly, until something calls `relocate`.

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Lite