docs: strengthen PET Rust review guidance (Fixes #496)

.github/agents/Reviewer.agent.md

@@ -35,6 +35,10 @@ Automated reviews consistently miss:
 - Thread safety issues with shared state
 - JSONRPC protocol violations (stdout contamination)
 - Performance regressions from spawning Python processes
+- Path-keyed caches that normalize keys inconsistently or return stale caller-facing paths
+- Unicode-unsafe byte indexing and unnecessary hot-path allocations
+- Tests that execute code without proving the claimed I/O or allocation reduction
+- Duplicate warnings, reports, or telemetry emitted from overlapping paths
 
 ---
 
@@ -44,6 +48,9 @@ Automated reviews consistently miss:
 
 Before reading code:
 
+- If any changed file is Rust, load and apply `rust-coding-skill`.
+- Load `rust-locator-patterns` only when locator ordering, discovery, identification, path/symlink handling, or locator state is in scope.
+
 - What issue does this change claim to fix?
 - Which locator/crate is affected?
 - Does it touch identification logic (`try_from`) or discovery logic (`find`)?
@@ -162,6 +169,35 @@ let mut environments = self.environments
 - No deadlock potential from nested locks
 - Consider using `thread::scope` for structured concurrency
 
+### General Rust Correctness and Performance
+
+Apply `rust-coding-skill` to every Rust review, not only locator changes.
+
+**Path-keyed state:**
+
+- Are lookup, insert, remove, prune, and sync keys normalized consistently?
+- Does a cache hit preserve the current caller's user-facing path rather than leaking the first cached spelling?
+- Is there Windows coverage for equivalent casing or separators?
+
+**Parsing:**
+
+- Are byte offsets computed from the same string being sliced?
+- If markers are ASCII, does the code use byte-stable ASCII matching and checked `str::get` slicing?
+- Is there a non-ASCII regression case around paths or names?
+
+**Hot paths:**
+
+- Does the implementation read each metadata file once per logical operation?
+- Did the author trace root/base and manager call paths, not just the common environment path?
+- Are borrowed snapshots passed through instead of reopening files or cloning strings?
+- Can `rfind`, `find_map`, or streaming output replace an intermediate `Vec` or `String`?
+
+**Side effects and tests:**
+
+- Can a warning, notification, manager, or environment be emitted twice by pre-check and worker paths?
+- Does the test assert the claimed invariant (read count, cache hit, event count), not just the final value?
+- Are classification boundaries covered (`**` path segment vs `foo**bar`, valid vs malformed markers)?
+
 ### Platform-Specific Code
 
 **Use the correct conditional compilation:**

.github/skills/rust-coding-skill/SKILL.md

@@ -0,0 +1,88 @@
+---
+name: "rust-coding-skill"
+description: "Use whenever editing Rust in PET to write allocation-aware, cross-platform, byte-safe code with behavior-proving tests."
+---
+
+# PET Rust Coding Skill
+
+Use this alongside `rust-locator-patterns`. Priority order:
+
+1. Readable code with explicit invariants
+2. Correct cross-platform and concurrent behavior
+3. Measured performance improvements without duplicate work
+
+## Path Identity and Caches
+
+Use `Path`/`PathBuf` for paths. Preserve the caller-facing path in reported values, but normalize cache and comparison keys with existing PET helpers such as `norm_case`.
+
+A normalized key does not imply the cached value can expose the first caller's spelling:
+
+```rust
+let key = norm_case(path);
+let mut cached = cache.get(&key)?.clone();
+cached.prefix = Some(path.to_path_buf());
+```
+
+When adding or reviewing a path-keyed cache, check lookup, insert, remove, retain/prune, and state-sync paths. Add Windows coverage using equivalent separators or casing; do not test only the happy-path spelling.
+
+## Byte-Safe Parsing
+
+Never calculate byte offsets from a transformed Unicode string and apply them to the original. Unicode case conversion can change byte length.
+
+For ASCII wire/file markers, use byte-stable ASCII-insensitive matching and checked slicing:
+
+```rust
+let marker = b"# cmd:";
+let start = line
+    .as_bytes()
+    .windows(marker.len())
+    .position(|window| window.eq_ignore_ascii_case(marker))?
+    + marker.len();
+let value = line.get(start..)?.trim();
+```
+
+Use `to_ascii_lowercase` rather than `to_lowercase` when the format is defined as ASCII. Add a non-ASCII path regression test whenever offsets are derived from textual markers.
+
+## Hot-Path I/O and Allocations
+
+Discovery runs frequently and in parallel. Before adding a cache, prove the repeated work and define invalidation. Within one operation, read immutable metadata once and pass borrowed snapshots through parsers.
+
+- Prefer `&str`/`&[u8]` over cloning content between parsers.
+- Prefer `rfind`/iterator operations over collecting an intermediate `Vec` just to select one item.
+- Avoid `format!` and Unicode case conversion in per-line loops when ASCII matching or direct writes suffice.
+- Do not claim an optimization is complete until every relevant call path is traced, including base/root environments and manager lookup.
+- Do not emit the same warning, telemetry event, or report from both a pre-check and the worker path.
+
+## Error Handling and Locks
+
+Library code should preserve typed information where repository APIs permit it. Prefer `?`, `let-else`, and `if let` over broad fallbacks. Do not swallow filesystem errors when doing so can make stale cache data look valid.
+
+Use contextual `expect` for poisoned locks in production code, matching the surrounding crate. Keep lock scopes short and never perform filesystem I/O or callbacks while holding a shared-state lock unless the design explicitly requires it.
+
+## Cross-Platform Semantics
+
+- Use `#[cfg(...)]` for platform-only code; `cfg!` does not prevent compilation.
+- Avoid `canonicalize` for Windows junction identity; use PET path helpers.
+- Treat both `/` and `\` as separators when parsing user patterns, but only classify `**` as recursive when it is a complete path segment. `foo**bar` is not a recursive segment.
+- Preserve original user-facing paths after normalized comparisons.
+- Prefer raw string literals for regexes and backslash-heavy path examples to avoid malformed escapes.
+- Before documenting or logging a recommended config value, trace how the consumer uses it. For example, `environmentDirectories` contains directories that hold environments, not environment folders themselves.
+
+## Tests Must Prove the Change
+
+Tests should demonstrate the behavior or performance invariant, not merely execute new lines.
+
+For optimizations, instrument the dependency boundary and assert the operation count:
+
+```rust
+let reads = AtomicUsize::new(0);
+parse_with_reader(path, |_| {
+    reads.fetch_add(1, Ordering::Relaxed);
+    Some(history.clone())
+});
+assert_eq!(reads.load(Ordering::Relaxed), 1);
+```
+
+For parser helpers, include malformed input, non-ASCII surrounding data, and case variations. For diagnostics, test pattern classification and expansion filtering separately. Keep temp paths unique with `tempfile` or process/counter-based names.
+
+Before every Rust commit, run targeted tests and invoke the `rust-precommit` skill. Keep that skill as the single source of truth for required format and Clippy commands.