docs: refine code-review skill with conformance, cache, and async checks

.agents/skills/review/SKILL.md → .agents/skills/code-review/SKILL.md

@@ -1,5 +1,5 @@
 ---
-name: review
+name: code-review
 description: Conduct authoritative code review on python-roborock changes against repository architectural hierarchy, typing standards, and maintainer conventions.
 ---
 
@@ -13,11 +13,13 @@ All reviews are evaluated against the engineering conventions defined in [AGENTS
 
 ## Review Workflow
 
-1. **Classify Changed Files by Layer**:
-   Group changed files according to the 3-tier architectural hierarchy:
+1. **Classify Changed Files & Scope**:
+   Group changed files according to the 3-tier architectural hierarchy and evaluate PR scoping:
    - **Priority 1: Public Trait APIs & Consumer Contracts** (`roborock/devices/traits/`, `roborock/data/containers.py`)
-   - **Priority 2: Data Lifecycle & State Management** (trait state, callbacks, `RoborockDevice.close()`)
+   - **Priority 2: Data Lifecycle & State Management** (trait state, callbacks, `RoborockDevice.close()`, map/render pipelines)
    - **Priority 3: Wire Protocols, Codecs & Cryptography** (`roborock/protocols/`, `roborock/devices/transport/`, `roborock/devices/rpc/`)
+   - **PR Scoping**: Flag PRs that mix unrelated subsystems or combine large protocol/parser additions with trait consumption when they should be split into focused PRs.
+   - **Automated Conformance**: Structural invariants (`RoborockBase` inheritance, `RoborockEnum` fallback members, `RoborockException` rooting, and trait boundary types) are enforced in `tests/conformance/`. Flag any unjustified allowlist exemptions added to `tests/conformance/` and focus deeper review energy on semantic, lifecycle, and design issues.
 
 2. **Evaluate Layer 1: Public Trait APIs & Consumer Contracts**:
    - Are trait models subclassing `RoborockBase` with `@dataclass`?
@@ -28,9 +30,11 @@ All reviews are evaluated against the engineering conventions defined in [AGENTS
    - Are trait abstractions consumer-agnostic (not hardcoded exclusively to Home Assistant entities)?
    - Are transport details (keys, tokens, sockets) cleanly isolated from traits?
 
-3. **Evaluate Layer 2: Lifecycle, State & Simplification**:
+3. **Evaluate Layer 2: Lifecycle, State & Concurrency**:
    - **Value-Returning Helpers**: Do helper functions return values instead of modifying `self` via hidden side effects?
    - **Direct Flow**: Is the execution path straightforward and direct, avoiding unnecessary indirection or ping-ponging between methods?
+   - **Render & Cache Immutability**: Are cached telemetry state and raw packets kept immutable during presentation/rendering? When caching derived state (e.g., parsed maps or calibration), is cache invalidation handled accurately across out-of-order or concurrent updates?
+   - **Event Loop Safety**: Are CPU-intensive operations (e.g., heavy map decompression or PIL image rendering) kept off the `asyncio` event loop (such as via `asyncio.to_thread`) with proper concurrency/staleness guards?
    - **Concurrency**: Does 1:1 async command-response matching use `asyncio.Future` correlated by protocol request/sequence ID (`request_id` or `msg_id`)?
    - **Teardown**: Are all tasks, event listeners, and channels cleanly unhooked in `RoborockDevice.close()`?
    - **Capability Gating**: Is capability gating checking protocol version, category (appropriate for device family), and device feature flags?
@@ -47,8 +51,6 @@ All reviews are evaluated against the engineering conventions defined in [AGENTS
    - Do assertions verify public behavior rather than private `_state` variables?
 
 6. **Format Review Output**:
-   Structure review comments with clear rationale and actionable code recommendations:
-   - **Priority Level & File Reference**
-   - **Issue / Anti-Pattern Identified**
-   - **Grounded Rationale** (citing protocol resilience, consumer stability, or maintainability)
-   - **Concrete Before / After Diff or Suggested Fix**
+   Keep feedback high-signal: focus on architectural violations, state/concurrency bugs, protocol safety, and test gaps; avoid nitpicking formatting or style already enforced by Ruff and Mypy.
+   - **Interactive / Summary Reviews**: Group findings by Priority Layer (1 → 3) with file references, the anti-pattern identified, grounded rationale (citing protocol resilience, consumer stability, or maintainability), and a concrete before/after diff or suggested fix.
+   - **Inline PR Review Comments**: State the issue, grounded rationale (citing `AGENTS.md` conventions), and suggested fix concisely right on the affected lines.

.github/skills

@@ -0,0 +1 @@
+../.agents/skills

AGENTS.md

@@ -97,5 +97,5 @@ Evaluate every change against the repository's three core layers in strict prior
 
 Portable agent skills following the [Agent Skills specification](https://agentskills.io/specification) are located under `.agents/skills/`:
 
-- **Code Review Skill** (`.agents/skills/review/SKILL.md`):
-  A structured, interactive review workflow for evaluating pull requests and diffs against the repository's architectural hierarchy, typing rules, and testing standards. To invoke during interactive sessions, refer to [.agents/skills/review/SKILL.md](.agents/skills/review/SKILL.md).
+- **Code Review Skill** (`.agents/skills/code-review/SKILL.md`):
+  A structured, interactive review workflow for evaluating pull requests and diffs against the repository's architectural hierarchy, typing rules, and testing standards. To invoke during interactive sessions, refer to [.agents/skills/code-review/SKILL.md](.agents/skills/code-review/SKILL.md).