neko-quality-review

name	neko-quality-review
description	Use after non-trivial code changes in Neko Suite, when reviewing PRs, or when asked to run a repository quality review. Applies the Neko code review and quality gates ADR to classify risk, inspect architecture boundaries, choose validation commands, and produce review findings with verification and residual risk.

Neko Quality Review

Use this skill after implementing or modifying code in this repository, and whenever the user asks for code review, quality review, PR readiness, CI readiness, UX/performance review, or architecture gate checks.

Source of truth:

• docs/architecture/adr-code-review-quality-gates.md
• docs/architecture/application-composition.md
• docs/architecture/package-boundaries.md
• AGENTS.md

Workflow

1. Inspect the change set:

   git diff --name-only
   git diff --stat
   

2. Classify risk:

   • L0: docs, copy, low-risk single-file fix.
   • L1: local component, hook, service, or state logic.
   • L2: renderer/preload/Main messaging, shared packages, media ports, imports/exports, public types.
   • L3: Rust engine, Proto, media streams, rendering, project formats, AI workflow, packaging.
   • L4: release, install/packaging, major UX, core creative workflow.

3. Review architecture before implementation details:

   • Does it fit the existing architecture?
   • How does it reduce coupling?
   • Is it easy to extend and test?
   • Is the complexity proportional to a local Electron Desktop plus local Node/FFmpeg runtime?
   • Does defensive code protect real boundaries instead of hiding development errors?
   • Do code defects and contract violations fail visibly instead of falling back, defaulting, or no-oping?

4. For multi-module changes or new functionality, apply the five-layer analysis:

   • Responsibility: who owns data, behavior, and lifecycle?
   • Dependency: are L0/L1/L2 and renderer/preload/Main/Node boundaries respected?
   • Interface: are types, messages, schemas, and Proto contracts minimal and stable?
   • Extension: will the next similar feature avoid broad edits or duplication?
   • Testing: what is covered by unit, integration, build smoke, an isolated Electron Desktop scenario, or explicit residual risk?

5. Run or recommend validation by impact:

   pnpm ci:local
   

   For Rust changes:

   pnpm ci:local:rust
   

   For Proto changes:

   pnpm ci:local:proto
   

   If a narrower package command is enough, prefer the smallest reliable command and state why.

   For residual/debt and redundancy checks:

   pnpm check:legacy-debt
   pnpm check:unused
   

   For integration smoke checks:

   pnpm smoke:webview
   pnpm package:desktop
   

   For Agent Evaluation platform, scenario, debug automation protocol, or exported fact-contract changes, run the key-free harness gate documented by the platform. This is not real Agent behavior acceptance. If the change can alter prompt or Skill behavior, capability/tool registration or routing, provider/model selection, AgentSession multi-turn/queue/async/recovery behavior, or Desktop Agent event projection, use neko-agent-evaluation to produce focused path-level evidence. Review the recorded blocking condition and residual risk when a real case could not run. Do not infer that scenario assertions passed unless the current runner executed an evaluator for them.

   For implemented user-visible UI behavior, use neko-ui-validation to derive the affected function inventory and produce focused functional, visual, and adjacent-regression evidence. Treat a failed or blocked UI result as an advisory finding or follow-up, not a blocking code finding, unless the same defect is independently established by a functional, contract, security, or code-gate failure. Renderer/Webview behavior that crosses focus, CSP, media preview, host messaging, native resources, or lifecycle boundaries should still use an isolated Electron Desktop fixture through the production package or controlled app runtime; browser-only evidence cannot prove those boundaries.

Review Checklist

Always check:

• No new production any, unsafe as Type, or formal logging via console.log.
• No circular dependency or broken layer direction.
• No overdesign for the local-client/local-engine product boundary: avoid speculative interfaces, factories, registries, strategies, plugin hooks, feature flags, config layers, protocol layers, or generic platform scaffolding without a current caller, real external provider, release, or trust boundary.
• No overdefense that hides defects: broad try/catch, silent defaults, fallback success, repeated validation, no-op guards, retries, caches, or circuit-breaker-style logic must protect a real Desktop renderer/preload/Main, local file, Node/FFmpeg process, media, external provider, user-data, release, or security boundary and fail visibly for development errors.
• Fail-visible defect handling is enforced: missing new implementations, contract mismatches, unreachable states, illegal messages, unknown schema/version values, bad configuration, missing dependencies, or unregistered handlers/renderers/adapters throw, return typed diagnostics, or fail tests visibly instead of returning empty data, success defaults, no-ops, silent degradation, or old compatibility behavior.
• Renderer/Webview code does not import Electron, Node APIs, or Desktop Main/preload implementations.
• Desktop Main code does not import React/ReactDOM or Webview implementations.
• TypeScript does not duplicate Rust engine authoritative computation.
• Paths are relative or ${VAR}/path, not hard-coded absolute paths.
• File/document/media/model/thumbnail/preview/proxy/import/export/transfer changes use the owning shared boundary: content access, resource cache, local resource access, EngineClient/file access, path resolver, ingest, or project-file service. Feature packages provide provider/adapter/domain semantics; they do not create package-local cache managers, path resolvers, Webview URI projectors, Engine file-token policies, or cache manifest readers.
• Cache is transparent and rebuildable: business logic, Agent tools, Webviews, Canvas nodes, Storyboard rows, composite artifacts, clipboard payloads, and cross-plugin transfer payloads must not use .neko/.cache layout, cache manifests, materialized cache paths, cachePath, runtimePath, cacheResourceRef, Webview URI, blob/object URL, Engine token, preview token, or scratch/temp paths as durable identity.
• Webview-safe URIs are produced only by LocalResourceAccessService or ResourceCacheService.project() after authorization. Projection failure returns typed diagnostics, omits renderable projection, or fails closed; it does not fall back to raw local/cache/source paths.

Add domain checks as needed:

• Webview/UX: component reuse audit, layout, theme, focus, keyboard, i18n, and runtime evidence from an isolated Electron Desktop fixture; browser-only screenshots do not count as Desktop IPC/lifecycle acceptance evidence.
• UI reference review: when run, inspect the neko-ui-validation applicability decision, acceptance inventory, authoritative runtime, functional and visual results, adjacent regression evidence, and residual risk. Report findings accurately without making this advisory review a code gate.
• Engine/media: cargo test, CLI smoke, serve integration, performance before/after when relevant.
• Proto/shared: generated types are synchronized and callers are migrated.
• Agent/AI: tool contracts, permissions, Journal/traceability, failure recovery, and whether the change triggers neko-agent-evaluation. When triggered, check focused canonical-path evidence, forbidden-fallback evidence, assertion support in the current runner, and either a real Desktop complete-session result or an explicit blocking condition with residual risk. Protocol-only, mock-only, or final-text-only results do not count as Agent behavior acceptance.
• Content access/cache/path: intent-aware access, transparent resource cache, path variable resolution, Engine-backed binary/media reads, Host text/project-file reads, authorized Webview projection, and stable ContentLocator transfer.
• Assets/market: manifest/schema compatibility, path safety, cache invalidation, trust boundaries.

Output Format

For review findings, lead with issues:

Findings
- Blocking: [file:line] Problem. Impact. Suggested fix.
- Suggestion: [file:line] Problem. Impact. Suggested fix.

Verification
- Ran: ...
- Not run: ... (reason)

Residual Risk
- ...

If there are no findings, say so clearly and still report test gaps or residual risk.

For post-implementation self-review, summarize:

• Risk level and affected areas.
• Key architecture/contract decisions.
• Validation performed.
• Remaining follow-up, especially engine CLI smoke, serve integration, Webview runtime smoke, UX evidence, or performance baselines.