diff --git a/manifest.yaml b/manifest.yaml index f4d063c..9402ab2 100644 --- a/manifest.yaml +++ b/manifest.yaml @@ -207,6 +207,15 @@ protocols: boundaries, interior mutability, and resource leaks. language: Rust + - name: clone-discipline-rust + path: protocols/analysis/clone-discipline-rust.md + description: > + Rust review protocol for challenging unnecessary or high-cost + cloning. Covers borrow-first alternatives, loop and closure clone + hotspots, Option/Result extraction patterns, and false-positive + suppression for deliberate ownership boundaries. + language: Rust + - name: thread-safety path: protocols/analysis/thread-safety.md description: > @@ -597,6 +606,14 @@ protocols: verification, cross-component compatibility, and decision matrix generation. Scoped to core functional components. + - name: design-context-mapping + path: protocols/reasoning/design-context-mapping.md + description: > + Pre-review reasoning protocol for discovering repository standards + and mapping changed modules to relevant design documents. Covers + standards inventory, design-doc discovery, file-to-doc matching, + and explicit handling of gaps when no governing design exists. + - name: schematic-design path: protocols/reasoning/schematic-design.md description: > diff --git a/protocols/analysis/clone-discipline-rust.md b/protocols/analysis/clone-discipline-rust.md new file mode 100644 index 0000000..47479bf --- /dev/null +++ b/protocols/analysis/clone-discipline-rust.md @@ -0,0 +1,148 @@ + + + +--- +name: clone-discipline-rust +type: analysis +description: > + Rust review protocol for challenging unnecessary or high-cost cloning. + Covers borrow-first alternatives, loop and closure clone hotspots, + Option/Result extraction patterns, and false-positive suppression when + ownership-preserving clones are the clearer design. +language: Rust +applicable_to: [] +# User-composed protocol — not auto-included by any template. +# Intended for: Rust code review, PR review, and maintainability audits +# where ownership workarounds may hide better borrowing or sharing designs. +--- + +# Protocol: Clone Discipline Review (Rust) + +Apply this protocol when reviewing Rust code for unnecessary or +high-cost cloning. The goal is to distinguish clones that preserve a +clear ownership boundary from clones that merely silence the borrow +checker or duplicate expensive data without need. + +## Deterministic Baseline + +If deterministic tool output is available (for example, Clippy warnings +such as `clippy::redundant_clone`), treat it as a baseline input to the +review: + +1. Record the lint findings first and carry them forward as direct + evidence. +2. Do NOT stop at the lint output. Continue the semantic review for + borrow-vs-owning decisions, loop-local cloning, `Option` / `Result` + extraction patterns, repeated last-use clones, and API-shape issues + that deterministic lints may miss. +3. If the lint and semantic review disagree, cite both and explain why + the broader ownership analysis changes the conclusion. + +## Phase 1: Clone Inventory + +Build an explicit inventory before judging any clone site. + +1. Enumerate each `.clone()` call added or materially modified in the + reviewed scope. +2. For each site, record: + - The cloned type + - Whether the site is on a hot path (loop, retry path, async fan-out, + per-request handler) + - Whether the clone is on owned data, `Option`, `Result`, + collection types, or shared ownership wrappers (`Arc`, `Rc`) +3. Group related clones that serve the same purpose so they can be + judged together rather than as isolated lines. + +## Phase 2: Borrow-First Necessity Check + +For each clone site, determine whether borrowing or view-based access +would preserve the intended behavior. + +1. Ask what ownership transition the clone is enabling: + - Passing read-only data to a callee + - Iterating a collection + - Accessing data inside `Option` / `Result` + - Capturing data inside a closure or async block +2. Check for a borrow-based alternative: + - Pass `&T`, `&mut T`, `&str`, or slices instead of cloning owned data + - Iterate with references (`iter`, `iter_mut`, `for x in &items`) instead + of cloning the whole collection + - Use `as_ref`, `as_mut`, `as_deref`, pattern matching, or references + into `Option` / `Result` rather than cloning to call `unwrap` + - Prefer view conversions (`as_str`, `as_slice`) when only inspection + is required +3. Flag the clone when a borrow-first alternative exists and does not + change the surrounding ownership contract. + +## Phase 3: Cost and Frequency Assessment + +Not all clones have the same impact. Evaluate runtime and maintenance +cost, not just syntax. + +1. Classify the cloned value: + - **Low-cost**: trivially small structs, handles, shared ownership + wrappers cloned intentionally via `Arc::clone` / `Rc::clone` + - **Potentially high-cost**: `String`, `Vec`, `HashMap`, large structs, + buffers, ASTs, config graphs, request/response payloads +2. Increase severity when the clone occurs: + - Inside loops or iterator pipelines + - In async fan-out or repeated task spawning + - On error-retry paths or frequently invoked request handlers + - Multiple times along the same call chain for the same value +3. If the data must be read from multiple spawned tasks without mutation, + check whether shared ownership (`Arc` plus `Arc::clone`) is the + correct design instead of deep-cloning the payload. + +## Phase 4: Ownership-Structure Review + +Determine whether the clone is compensating for a deeper API or data-flow +problem. + +1. Check whether the callee takes owned input when it only reads data. + If so, recommend tightening the callee signature before accepting the + clone at call sites. +2. Check whether the clone exists only because a function mixes unrelated + responsibilities or holds a borrow for too long. If yes, note the + structural cause, not just the local clone. +3. When several sibling functions repeat the same clone pattern, treat it + as an API design issue rather than independent local nits. + +## Phase 5: False-Positive Suppression + +Do NOT flag every clone. Suppress findings when the clone is a deliberate, +clear ownership boundary and the alternatives would be worse. + +1. Do not flag clones that would require pervasive lifetime propagation + across unrelated APIs or stored structs merely to avoid a single, + localized owned value. +2. Do not flag intentional shared-ownership clones written as + `Arc::clone(&value)` or `Rc::clone(&value)` when the underlying data is + being shared, not duplicated. +3. Do not flag clones used to establish durable ownership at external + boundaries (config loading, FFI conversion, message handoff) when the + receiving side must own the data. +4. Downgrade or suppress findings for cheap clones when the alternative is + materially less readable and no hot-path or structural cost exists. + +## Output Format + +For each finding, report: + +```text +[SEVERITY: High|Medium|Low] +Location: : or +Clone site: +Issue: +Better pattern: +Evidence: +Confidence: +``` + +## Review Heuristics + +- Prefer owned clones at clear subsystem or thread boundaries over + lifetime-heavy signatures that spread borrow complexity everywhere. +- Prefer `Arc::clone` / `Rc::clone` spelling when shared ownership is + intentional; it makes review intent obvious. +- Treat repeated clones as a design smell first and a micro-optimization + concern second. diff --git a/protocols/reasoning/design-context-mapping.md b/protocols/reasoning/design-context-mapping.md new file mode 100644 index 0000000..5245834 --- /dev/null +++ b/protocols/reasoning/design-context-mapping.md @@ -0,0 +1,135 @@ + + + +--- +name: design-context-mapping +type: reasoning +description: > + Pre-review reasoning protocol for discovering repository standards and + mapping changed modules to relevant design documents. Covers standards + inventory, design-doc discovery, file-to-doc matching, and explicit + handling of gaps when no governing design exists. +applicable_to: [] +# User-composed protocol — not auto-included by any template. +# Intended for: code review, pull request review, and design-alignment +# audits that need repo-local standards and architecture context. +--- + +# Protocol: Design Context Mapping + +Apply this protocol before reviewing code that may be governed by +repository-specific standards or design documents. The goal is to ground +the review in the project's own rules and intended architecture instead of +reviewing the diff in isolation. + +## Phase 1: Standards Inventory + +Identify repository-local standards that define review expectations. + +1. Search the repo root and documentation roots for standards-bearing + files such as `CONTRIBUTING`, `README`, `GEMINI.md`, `CLAUDE.md`, + architecture guides, coding-standard docs, and reviewer playbooks. +2. For each candidate file, determine whether it is: + - **Mandatory**: explicit "must", "always", policy, CI gate, or + contribution rule + - **Advisory**: guidance, style preference, or example-based practice +3. Extract only the rules relevant to the review task: + - Build/test expectations + - Language-specific coding rules + - Architectural constraints + - Review-specific requirements (tests, docs, commit hygiene, safety rules) +4. Produce a standards list with path, authority level, and the exact + review behaviors it governs. + +## Phase 2: Design Document Discovery + +Discover the documents that describe intended behavior or architecture. + +1. Search likely design locations such as `designs/`, `docs/design/`, + `docs/architecture/`, `rfcs/`, `architecture/`, and subsystem-specific + design folders. +2. Record each candidate document's scope: + - Whole-system architecture + - Subsystem design + - Feature-specific behavior + - Workflow or operational design +3. If an index or umbrella design document exists, read it first to avoid + missing the repository's own terminology and document hierarchy. +4. Exclude stale or irrelevant docs only with evidence: + - Superseded by a newer named document + - Clearly unrelated subsystem + - Historical/archive material marked obsolete + +## Phase 3: Change-to-Context Mapping + +Map the changed files or modules to the most relevant standards and +design documents. + +1. Build a change inventory: + - Changed file paths + - Named modules, packages, or subsystems touched + - Public interfaces, configs, or workflows affected +2. For each changed area, score candidate context matches: + - **Direct match**: exact subsystem, feature name, or owning directory + - **Partial match**: same architecture layer or adjacent module + - **No match**: no design doc or standard clearly governs the change +3. Use multiple signals when scoring: + - Path overlap + - Shared terminology in headings or section titles + - Explicit references between code and docs + - Ownership or boundary descriptions in architecture docs +4. If multiple docs conflict, record the conflict and prefer the newest + or most specific document only when the evidence is explicit. + +## Phase 4: Context Loading Strategy + +Load only the context needed to review accurately. + +1. Read the highest-confidence standards and design docs for each changed + area before starting substantive review. +2. When a broad architecture doc exists plus a feature doc, read both: + architecture first for boundaries, feature doc second for intent. +3. If no matching design doc exists: + - State that absence explicitly + - Continue the review against code, tests, and standards only + - Do not fabricate intended architecture from naming alone +4. If the available context is too broad, prioritize sections that define: + - Invariants + - API contracts + - State transitions + - Resource ownership + - Expected tests or validation signals + +## Phase 5: Review Framing Output + +Produce a compact context map before the main review. + +1. Summarize which standards files govern the review and what rules they + impose. +2. Summarize which design docs map to which changed modules. +3. Record uncovered areas: + - Changed code with no governing design doc + - Design docs found but not read due to scope limits + - Conflicts or ambiguity requiring user judgment +4. Use this map to drive later design-alignment findings. If a change + violates a mapped design, cite the exact document section. If no design + exists, do not report a design-alignment violation. + +## Output Format + +Produce a context map in this form before the main review: + +```markdown +## Review Context Map + +### Standards +| Path | Authority | Relevant Rules | +|------|-----------|----------------| + +### Design Mapping +| Changed Area | Matched Document | Match Strength | Why | +|--------------|------------------|----------------|-----| + +### Gaps and Ambiguities +- +```