---
name: personal-assistant-review
description: Review uncommitted or PR diffs in the personal-assistant Obsidian plugin with project-specific risk lanes, second-layer future-risk checks, severity discipline, subagent review routing, and validation boundaries. Use when the user asks for code review, agent team review, multi-angle review, review-first analysis, code-level release-readiness review, hidden side effects, compatibility risk, edge cases, security/performance risk, test gaps, maintenance cost, or comparison of review quality in this repository. For a cross-surface UI/UX design audit, use `ui-ux-design-audit`; for stable or beta release preparation, use the matching release skill.
---

# Personal Assistant Review

## Core Rule

Use this skill for review-only or review-first work in the
`personal-assistant` repository.

Default to high-signal review. Find correctness, product-contract, state,
privacy, concurrency, docs/tests consistency, and release-blocking risks before
polish. Treat green checks and working happy paths as supporting evidence, not
closure. Do not invent findings to satisfy a role or lane.

## Start

1. Treat the task as review-only unless the user explicitly asks to fix.
2. Bound the current surface:
   - `git status --short --branch`
   - `git diff --stat`
   - `git diff --name-only`
3. Read the requirements/current contract, targeted diffs, and necessary
   dependencies before the implementer's conclusions. Form an independent
   view, then reconcile their explanation and validation evidence.
   For PA Agent/runtime/command changes, first check the [Command Architecture Contract](../../../docs/architecture/pa-agent-architecture-plan.md#command-architecture-contract)
   against the task's owner/factual-basis mapping and real call chain. Host
   admission must use explicit authority or execution facts, not prose intent;
   correctable attempts and accepted/unknown effects must remain distinct.
   This is a normal gate check before green-test acceptance, not optional polish.
4. Lead the final answer with findings ordered by severity.
5. State validation run and validation not run.

## Mode Selection

Use **gate mode** by default. This matches prompts such as:

```text
启动agent team对代码修改进行review
review current diff
release-readiness review
```

Gate mode optimizes for accurate P0/P1/P2 findings and direct fixability.

Use **exploration mode** when the user explicitly asks for many perspectives,
for example:

```text
从程序员、架构师、产品经理、UI/UX、性能、desktop&mobile多端支持等角度评审
多角度体检
全面扫描产品、架构、UI 和性能
```

Exploration mode may include P3 design debt and polish, but still must not
force a finding per role.

Use **hybrid mode** for broad diffs when unsure: run gate mode first, then add
optional exploration findings under a separate `Optional Polish` section.

## Subagent Lanes

Select lanes from the actual diff and its affected dependencies; the examples
below are not mandatory read sets. Use independent subagent lanes when requested
or when parallel work saves time or improves coverage. Keep a narrow review
local when splitting adds no value; if tools are unavailable, cover the relevant
risks locally. Docs/skills-only reviews need no unrelated Pagelet/runtime lane.

Assign bounded risk questions without requiring a finding from each lane.
Follow `AGENTS.md` **Multi-Agent Validation Coordination**: the main agent
merges evidence and schedules expensive gates; reviewers report findings and
focused evidence without each running full tests/build/deploy. Review-only
lanes do not edit files, including while a final gate is running.

### Gate Mode Lanes

1. **Functional state and concurrency**
   - Inspect Pagelet orchestration, foreground run guards, stale result
     handling, save state, panel/tab routing, command aliases, hidden side
     effects on old state, and tests.
   - Focus files: `src/pagelet/orchestrator.ts`,
     `src/pagelet/AnalysisSessionManager.ts`, `src/pagelet/ReviewNoteSaveFlow.ts`,
     `src/pagelet/BubbleCoordinator.ts`, `src/pagelet/commands.ts`,
     `src/pagelet/panel/PanelView.ts`, `src/pagelet/tab/*`,
     touched Pagelet tests.

2. **Spec, docs, tests, i18n, product contract**
   - Compare runtime behavior against changed docs and SDD/plans.
   - Inspect docs that changed plus relevant contract docs, especially
     Pagelet async-result, write-action, release, Memory/VSS, and tracker docs.
   - Check whether tests encode a behavior that contradicts a product/privacy
     non-goal.
   - Keep deterministic harness, actual-model semantic/recovery and real-app
     evidence distinct; none substitutes for another changed boundary.
   - Check whether tests cover failure, compatibility, stale-state, and
     concurrency edges, not only the easiest success path.

3. **Obsidian/community compatibility and lifecycle**
   - Scan for runtime DOM injection blockers:
     `createElement('style')`, `innerHTML =`, `outerHTML =`.
   - Inspect timers, listeners, observers, root unmount, CSS scope, public
     exports, old settings, command ids, storage scopes, generated assets,
     packaging/deploy impact, and compatibility-sensitive APIs.

4. **UI/UX/accessibility/mobile**
   - Inspect focus management, keyboard behavior, ARIA, mobile overflow,
     touch gestures, text clipping, theme variables, and ordinary-user copy
     (follow **Memory/VSS Product Rules** from AGENTS.md for user-facing terms).
   - Keep UI findings concrete: include the visible symptom or user flow.

5. **Performance, safety, and maintainability**
   - Inspect repeated scans, render loops, large-vault behavior, sync waits,
     background work, unnecessary provider calls, and teardown/unload behavior.
   - Inspect note text exposure, provider output persistence, file writes,
     external links, and command-triggered AI work for security/privacy risk.
   - Inspect misleading names, helper boundaries, and abstractions that could
     cause future callers to use an API incorrectly.

### Exploration Mode Lanes

Use the gate lanes, then add perspectives only where relevant:

- programmer/correctness
- architect/module boundaries
- product manager/workflow and value
- UI/UX/accessibility
- performance/rendering
- desktop/mobile support
- security/privacy
- naming/maintenance cost

Do not let these labels create coverage pressure. If a lane has no actionable
issue, say so.

## Maintainability And Probe Reliability

- For a new module or complex state change, walk through one plausible change
  adjacent to the current requirement. Locate the entry point, state owner,
  and invariants a future maintainer must preserve without the implementation
  conversation. Use the exercise to expose concrete coupling or misleading
  APIs; do not implement the hypothetical feature or require speculative
  abstractions. Apply the existing severity rules to any resulting finding.
- For changed tests/checkers/probes, select the risks they actually exercise:
  semantic comparison rather than incidental object-key order, text/frontmatter
  boundaries, async cleanup, repeatability, and expectations derived from the
  contract rather than copied implementation assumptions. Do not turn this
  list into a mandatory test matrix for every feature.
- Before relying on a new or materially changed probe's verdict, inspect the
  smallest normal and counterexample evidence for its relevant assertion.
  It must accept valid behavior and reject the failure it claims to detect.
  Missing evidence is a validation gap; review-only work reports it without
  writing fixtures or changing production code to satisfy the checker.

## Second-Layer Risk Lens

After the normal gate pass, assume the changed code compiles and the happy path
works. Before closing the review, ask what could still become a future bug:

- hidden side effects on old state, existing commands, settings, persisted data,
  generated assets, or unrelated views
- compatibility breaks across Obsidian versions, mobile/desktop, old plugin
  settings, storage scopes, command ids, public exports, and release packaging
- edge cases not covered by tests: failure paths, retries, empty data, stale
  async results, concurrent runs, teardown/unload, degraded providers, and large
  vaults
- performance risks from repeated scans, render loops, sync waits, background
  work, unnecessary provider calls, or memory growth
- security/privacy risks around note text, provider output, persisted state,
  file writes, external links, and command-triggered AI work
- misleading names, helper boundaries, or abstractions that make future callers
  likely to use the API incorrectly
- tests that only prove the easiest success path and do not pin product,
  privacy, compatibility, or lifecycle behavior

Every second-layer finding still needs a concrete trigger path, code reference,
or verifiable assumption. If the concern is plausible but not proven, label it
as `P3`, `needs decision`, or `optional polish` instead of presenting it as a
blocker.

## Project Risk Checklist

Use this checklist to guide search, not to manufacture findings.

Pagelet:

- `sourcePath` vs `primarySourcePath` vs `saveFlow.pending.targetPath`
- `currentPanelLayout` and discovery/summary/review routing
- `PanelView.close()` / `onClose` clearing state unexpectedly
- stale foreground review results and pet/panel state transitions
- `beginForegroundReviewRun()` consistency across review, discovery, summary,
  and other provider-backed foreground work
- pending generated markdown or provider output persisted in Obsidian view
  state, workspace state, settings, or vault files without explicit approval
- `resolveRelatedMarkdownNote` / `getFirstLinkpathDest` source-path context
- bubble quick actions that start AI/provider work before clear data/cost/scope
  disclosure
- keyboard focus restoration from bubble, panel, and native detail tab
- SVG graph keyboard/touch behavior and label clipping
- legacy command aliases and command labels

Memory/VSS:

- durable vs fallback behavior
- VSS operation queue / exclusive lock for mutating operations
- dirty state, background reconcile, retry/backoff, and chat non-blocking paths
- user-facing copy per **Memory/VSS Product Rules** from AGENTS.md

Community/release:

- no runtime `<style>` injection
- no `innerHTML` / `outerHTML` assignment in plugin DOM code
- generated `styles.css` matches Tailwind source for CSS changes
- release docs and links point to existing files or clearly future assets
- package/deploy paths include any new runtime assets

## Severity Rules

Use severity to reflect user impact and release risk.

- **P0**: data loss, source-note corruption, security/privacy breach, plugin
  unusable on common path.
- **P1**: release-blocking correctness or product-contract violation, especially
  privacy/data persistence, explicit SDD non-goal violation, broken write guard,
  or community review `Error`.
- **P2**: must-fix before merge/release for likely user-visible failure,
  state/concurrency bug, wrong file write/open, broken docs link in release
  material, serious accessibility/focus problem.
- **P3**: optional polish, maintainability debt, visual refinement, performance
  concern without clear user-visible failure.

If a finding depends on a design decision rather than a definite bug, label it
as `needs decision` or `optional polish`. Do not escalate vague future risk to a
blocker unless there is a likely user-visible, privacy, data-safety,
compatibility, or release impact.

## Validation

For code/DOM changes, run the **Local Validation Gate** from AGENTS.md, scoped to
the changed surface. Use its docs/skills-only checks for instruction changes.
For broad Pagelet or shared-runtime diffs, also include `npm run lint`.

Apply `AGENTS.md` **Validation Planning And Reuse** and **Test Failure Diagnosis**;
reuse checks with verified inputs/results and request only missing evidence
for a concrete risk. The main agent coordinates expensive gates as above.

Per AGENTS.md Testing Instructions, do not claim Obsidian validation without
deployed evidence. If `make deploy` and real test-vault smoke were not run, state it.

## Output

Use this structure:

```markdown
**Findings**
- P1/P2/P3 [file](/absolute/path:line): concise issue.
  Impact/trigger. Suggested fix.

**Validation**
- checks run
- checks not run / residual risk

**Notes**
- optional polish, second-layer risks, test gaps, or needs-decision items, only
  when useful
```

Keep summaries secondary. If there are no actionable findings, say that clearly
and list remaining validation gaps.

## Related Skills

- To triage and implement fixes from this review, use `personal-assistant-review-followup`.
- For cross-surface UI/UX design audits and visual baselines, use `ui-ux-design-audit`.
- For app-level smoke validation, use `obsidian-test-vault-smoke`.
- For real-device iOS validation, use `obsidian-ios-real-device-smoke`.
- For community compliance scan before release, use `obsidian-community-check`.