arch-code-review

name	arch-code-review
description	Architecture-aware review against canonical structured YAML artifacts. Read-only workflow; no legacy prose docs.

Architecture Code Review — Structured API Only

Goal: review diffs for correctness, drift, data consistency, side-effect boundaries, contract compatibility, and tests using YAML artifacts from the resolved structured docs root.

Structured Artifact API Contract

Legacy prose artifacts are deprecated. Do not create, update, or rely on docs/agent/*.md, scoped prose docs, or generated human-readable Markdown views. Use structured YAML for canonical artifacts. Root AGENTS.md remains a harness interoperability file and may be generated/updated only by workflows that explicitly say so.

Resolved structured docs root:

Treat docs/agent/api as a logical layout rooted at a resolved structured docs root, not a fixed repo path.

Resolution rules:

1. Resolve workspace_root with git rev-parse --show-toplevel 2>/dev/null or fallback to pwd.
2. Canonicalize workspace_root before fingerprinting when possible (realpath, pwd -P, Path(...).resolve(), or equivalent).
3. safe-start always creates and uses the initial repo-local root: <workspace_root>/docs/agent/api.
4. Review reads repo-local only when <workspace_root>/docs/agent/api already exists.
5. Otherwise read the global overlay root: ~/.pi/agent/workspaces/<workspace-fingerprint>/docs/agent/api.
6. Compute <workspace-fingerprint> exactly from canonical workspace_root: strip one leading slash/backslash, replace every slash, backslash, and colon with -, then wrap with --. This keeps the same workspace stable.
7. Example: /data/data/com.termux/files/home/CodeProjects/pi-mono -> --data-data-com.termux-files-home-CodeProjects-pi-mono--.
8. Do not create new repo-local structured docs in unadopted repos unless the user explicitly asks for repo-local adoption there.

Logical structured layout under the resolved docs root:

repo/
  scopes.yaml
  repo-inventory.yaml
  project-intent.yaml
  architecture.yaml
  data-flow.yaml
  data-model.yaml
  invariants.yaml
  dependency-rules.yaml
  design-issues.yaml
  risk-register.yaml
  change-guide.yaml
  testing-strategy.yaml
  validation-baseline.yaml
  contracts.yaml
  adr.yaml
  agent-operating-guide.yaml
scopes/
  by-path/<repo-relative-path>/...
  by-domain/<slug>/...

Every structured artifact must conform to ../_shared/references/schemas/common.schema.json plus its artifact-specific schema. Do not inline, invent, or vary envelope fields.

Stable IDs required: scope:*, component:*, entity:*, invariant:*, risk:*, contract:*, flow:*, command:*, issue:*, adr:*, testplan:*.

Ownership rules:

• scopes: scope routing, ownership, cross-scope discovery only.
• repo-inventory: file tree, commands index, entry points, external boundaries, configs.
• validation-baseline: command status, blockers, recommended validation order.
• project-intent: goals, users, journeys, non-goals, constraints, assumptions.
• architecture: components, architecture style, side-effect boundaries, high-level flow refs.
• data-flow: typed flow graph/steps, inputs, outputs, error states.
• data-model: entities, IDs, schemas, relationships, lifecycles, serialized formats.
• invariants: rules, forbidden states, enforcement locations, invariant-test refs.
• dependency-rules: layers, allowed/forbidden dependencies, violations, coupling hotspots.
• design-issues: structural drift, deferred decisions, ambiguity, ownership gaps.
• risk-register: failure modes, severity/confidence, affected refs, suggested tests/fixes.
• contracts: cross-scope APIs, schemas, events, generated clients, DB/file/deployment/env contracts.
• testing-strategy: test topology, coverage gaps, risk-to-test priorities.
• change-guide: workflow routing and checklists; references owner artifacts, duplicates no facts.
• adr: structured decision records with bounded prose fields.
• agent-operating-guide: structured source for agent operating rules. Root AGENTS.md may mirror this in compact harness-readable Markdown when produced by safe-start or codebase-recon Pass 6.

Redundancy rule: define each fact in its owner artifact exactly once. Other artifacts reference IDs. Prose rule: bounded prose allowed only in summary, notes, rationale, context, decision, recommended_action, and similar scalar fields. Scope rule: if focus is path-like, read under <docs-root>/scopes/by-path/<focus>/; otherwise under <docs-root>/scopes/by-domain/<slug>/. Always resolve scope from <docs-root>/repo/scopes.yaml when present.

Runtime Schema Loading

When a workflow creates, updates, migrates, or validates structured artifacts, read ../_shared/references/artifact-api.md first. Then read only the shared skill package schemas needed for the artifacts being written:

• ../_shared/references/schemas/common.schema.json
• ../_shared/references/schemas/<artifact-file-base>.schema.json

Do not read all schemas. Do not use templates. Schemas are runtime API contracts; project docs outside the shared runtime refs are maintainer aids unless the user asks about this package itself.

Structured Artifact Write/Update Protocol

Use this protocol whenever creating or updating YAML artifacts.

1. Scope and owner resolution

1. Resolve scope first from task/focus and <docs-root>/repo/scopes.yaml when present.
2. Path focus uses longest prefix match; domain focus requires explicit domain/contract/task evidence.
3. Select the single owner artifact for each fact using the ownership rules above.
4. Never duplicate owner facts in router/checklist artifacts; reference stable IDs instead.

2. Read-before-write

1. Read the existing target YAML if it exists.
2. Read directly referenced owner artifacts needed to preserve refs and avoid duplication.
3. If target YAML is absent, create it with the common envelope and artifact-specific top-level keys.
4. Preserve unknown fields unless they conflict with this protocol; do not silently drop agent/user-added structured data.

3. Stable ID generation

1. Reuse existing IDs whenever the semantic object is the same, even if name/path changed.
2. New IDs use deterministic slugs from owner scope + semantic name: risk:<slug>, entity:<slug>, component:<slug>, etc.
3. If two objects slug-collide, append shortest stable discriminator from path/component/contract, not a random suffix.
4. Never renumber IDs because order changed.

4. Upsert semantics

For each discovered fact/object:

1. Match existing record by ID first.
2. If no ID match, match by stable source-of-truth fields: path+symbol, contract source path, command string+cwd, entity name+owner scope, rule owner+kind.
3. If matched, update only changed fields, append/refresh evidence, and preserve unrelated fields.
4. If unmatched, insert new record in deterministic order by ID or explicit order field.
5. If an existing observed record is no longer supported, do not delete it by default. Mark status: stale or deprecated, add evidence/unknown explaining why, and link replacement ID when known.
6. Delete only records known to be accidental duplicates or malformed, and mention deletion in final response.

5. Evidence and confidence

1. Every observed record needs at least one evidence ref with file/symbol/command/doc/diff observation.
2. Planned records may use evidence_mode: planned and confidence low or medium.
3. Mixed records must separate observed fields from planned/assumed fields via evidence refs or unknowns.
4. Do not upgrade status: current or confidence high without source or command evidence.

6. Reference integrity

Before writing final artifacts:

1. Check every *_ref, *_refs, and depends_on ID points to a record in the same artifact set or is explicitly listed as external/unknown.
2. Prefer adding missing owner records as compact stubs over leaving dangling refs.
3. For cross-scope refs, ensure scopes.yaml and contracts.yaml identify owner/consumer relationship.
4. If ownership is ambiguous, create/update design-issues.yaml with kind: ownership_gap and reference it.

7. Status transitions

Allowed transitions:

• planned -> partial -> current
• current -> stale -> current
• current|stale|partial -> deprecated

Rules:

• current requires sufficient observed evidence for the represented scope.
• partial means useful but incomplete evidence.
• stale means contradicted by newer source evidence or missing source path.
• deprecated means superseded; include replacement_ref when known.

8. Deterministic formatting

1. Use YAML with two-space indentation.
2. Use stable top-level key order: envelope keys first, artifact-specific keys next.
3. Sort unordered arrays by id; keep ordered flow/checklist arrays by order.
4. Use null, [], or {} consistently rather than omitting required envelope fields.
5. Keep prose scalar fields concise; no long narrative blocks.

9. Validation before completion

Perform best-effort validation after writing:

1. Re-read changed YAML for parse/syntax sanity when practical.
2. Validate against the shared schemas by inspection/re-read: envelope keys, artifact-specific top-level keys, required arrays/items, stable ID prefixes, and obvious dangling refs.
3. Verify no legacy Markdown artifacts were created or updated by the workflow, except root AGENTS.md when explicitly produced for harness interoperability.
4. Report changed YAML files, validation performed, unresolved unknowns, and any intentionally stale/deprecated records.

Invocation

Use this skill directly or use /review-arch as the prompt-template shortcut for architecture-aware diff review.

Rules

• Read-only. Do not edit code or artifacts.
• Review current diff unless user specifies target.
• Do not read legacy prose Markdown docs.
• Read repo/scopes.yaml first if present; match changed paths by longest prefix; then read relevant owner YAML artifacts.
• Every finding needs severity, location, evidence, owner artifact refs, why it matters, suggested fix.
• Prioritize correctness, invariants, contracts, dependency rules, side-effect boundaries, known risks, and tests.

Review Checklist

• Diff matches architecture.yaml components/boundaries.
• Diff respects dependency-rules.yaml.
• Data/schema changes update data-model.yaml and contracts.yaml as owner artifacts.
• Flow/error-state changes update data-flow.yaml.
• Invariants in invariants.yaml remain enforced/tested.
• Known risks in risk-register.yaml are not worsened.
• Design drift in design-issues.yaml is addressed or not worsened.
• Tests align with testing-strategy.yaml and validation-baseline.yaml commands.
• Scope ownership and contract owner/consumer refs remain valid in scopes.yaml and contracts.yaml.

Output

1. Verdict: approve | approve with comments | request changes
2. Findings: severity, location, problem, evidence, owner refs, impact, suggested fix
3. Missing tests
4. Structured artifact updates needed
5. Architecture drift risk
6. Final recommendation