Skip to content

REVIEW.md

Latest commit

 

History

History
105 lines (63 loc) · 5.82 KB

REVIEW.md

File metadata and controls

105 lines (63 loc) · 5.82 KB
Raw

Review instructions

What Important means here

Reserve 🔴 Important for findings that violate the acceptance criteria of an active spec this PR touches (listed below). A bug that breaks behavior the spec requires is Important. Style, naming, and refactoring suggestions outside the acceptance criteria are 🟡 Nit at most.

🟣 Pre-existing applies to bugs that exist in the codebase but weren't introduced by this PR.

Always check

  • Every commit subject ends with (SPEC-ID) matching one of the active specs below.
  • Source-file changes carry // trace:SPEC-ID | ai:<tool> comments linking to a spec.
  • Files traced to a spec NOT listed below are out-of-scope churn — flag as 🔴 Important.

Active specs in scope for this PR (4)

EPIC-19: aida doctor: maintenance & migration commands (per-repo, hidden top-level)

Status: InProgress

Spec description (no explicit Acceptance section):

User-suggested 2026-05-09. Idea: a tree of repair/migration commands under aida doctor ... (hidden via #[clap(hide = true)] so it doesn't pollute daily aida --help).

Existing piecemeal pieces that would consolidate here:

  • aida cache rebuild (already exists)
  • aida db block verify (FR-281, just shipped)
  • migrate per-type counter_scope to global (no tool yet; needed by FR-271 for projects already on per-type)
  • repair stale block registrations (the FR-1..100 orphan we found 2026-05-09)
  • detect+fix dispenser collisions (BUG-31 fix prevents new ones; doesn't clean existing)
  • stale worktree pruning (already auto-runs in init via BUG-39)
  • migrate legacy YAML/SQLite to git-canonical (already exists as aida db export-git — could reroute under doctor migrate-storage)

Design questions to settle before implementation:

  1. Same repo (current preference per Claude's read) vs. separate repo. Same-repo + hidden subcommand keeps version-skew pain low.
  2. Should each operation be idempotent + dry-runnable by default? Yes — --dry-run everywhere.
  3. Scope: just data migrations? Or also "my project's broken, help me" runbooks? Probably both, but data-migrations land first since they're testable.

First implementation candidate: aida doctor migrate-counter-scope --to global — operates on a per-type project, computes the next free global counter floor (max across all type blocks), allocates a new * block above it, and writes counter_scope=global to config. Per-type blocks remain in place but no longer dispense from (find_active_block_or_global will hit the * block first... actually we'd need to retire per-type blocks too, so they don't keep dispensing FR-3 alongside the global counter's BUG-3).

Files in scope:

  • aida-cli/src/cli.rs
  • aida-cli/src/main.rs

EPIC-20: aida session start: scoped concurrent sessions with worktree + lease

Status: InProgress

Spec description (no explicit Acceptance section):

User-driven 2026-05-09. Goal: aida session start --owns <scope> gives a Claude Code (or terminal) session declared ownership over an EPIC / SPEC-ID / path scope, with a separate git worktree so file edits can't conflict between sessions running in parallel.

Files in scope:

  • aida-cli/src/cli.rs
  • aida-cli/src/main.rs

EPIC-21: Code↔store commit pairing: pin orphan store SHA per code commit

Status: InProgress

Spec description (no explicit Acceptance section):

User-driven 2026-05-09. The orphan store and the code repo evolve independently today. When you check out an old code commit, your code references reqs that the now-current store may have renamed/deleted. Trace comments dangle. Time-travel through history is broken — old branches don't "know" which store version they were written against.

Files in scope:

  • aida-cli/src/cli.rs
  • aida-cli/src/main.rs
  • aida-core/src/scaffolding/mod.rs
  • aida-core/templates/hooks/aida-store-pair.sh

TASK-583: SPIKE-35 v2 follow-ups: align bughunter-severity schema, regen stale REVIEW.md, drop thinking-comment

Status: InProgress

Spec description (no explicit Acceptance section):

Master review of spike-35-v2 (SPIKE-35 v2 + SPIKE-37) found 3 minor follow-ups — approved + merged anyway since none are blocking and the gate is functionally correct.

  1. Severity-schema mismatch: extract_bughunter_severity gate checks 'normal' (correct — real Code Review key) AND 'critical' (NOT a real key; inert dead code). Real documented schema is {normal, nit, pre_existing}. Drop the dead 'critical' gate-check; align test fixtures (currently use critical/cosmetic) to the documented normal/nit/pre_existing keys so tests validate against the real schema, not an assumed one. Ref: code.claude.com/docs/en/code-review check-run output.

  2. Stale v1 REVIEW.md: the root REVIEW.md committed in ebff464b (v1, one-global-file format) is still tracked. v2 assembles REVIEW.md from per-spec fragments at PR-open time, so the committed v1 content is stale until next assemble. Regenerate via 'aida review assemble' + commit, or remove and let /aida-pr assemble it.

  3. Leftover thinking-comment: rules_sync/main.rs delegated_reviewer_tests has a '// wait, ... is not valid JSON under standard serde_json' comment that reads as half-finished thought. Tidy.

Origin: master review 2026-05-29.

Files in scope:

  • aida-cli/src/main.rs

Skip

  • Generated files (per repo .gitignore); no need to flag formatting in build output.
  • Pure documentation rewording when no source-file changes accompany it.
  • CLAUDE.md / AGENTS.md edits unless they introduce contradictions with active specs.

Summary shape

Open the review with a one-line tally — e.g. 2 Important, 4 Nit, 0 Pre-existing — and if no Important findings exist, lead with No blocking issues against active spec criteria.