Add --updated flag to review-plan skill

[igerber]

Summary

• Add --updated flag to /review-plan skill that forces a complete fresh re-review of revised plans
• When --updated is provided, the agent performs all 8 review dimensions without short-circuiting, and appends a Delta Assessment section comparing against prior review feedback
• Flag can appear before or after the plan file path; missing path triggers the existing AskUserQuestion fallback

Methodology references (required if estimator / math changes)

• N/A - no methodology changes (skill prompt file only)

Validation

• Tests added/updated: No test changes (prompt-only change to .claude/commands/review-plan.md)
• Manual verification: flag parsing, argument-hint update, Step 1b insertion, Delta Assessment template, Notes bullet all confirmed via file read

Security / privacy

• Confirm no secrets/PII in this PR: Yes

---

Generated with Claude Code

[github-actions]

Overall Assessment

⚠️ Needs changes

Executive Summary

• Conflicting instructions on whether to include the Delta Assessment when --updated is set but no prior review exists; this makes the skill’s output ambiguous.
• --updated parsing is described as substring removal, which can corrupt valid plan paths containing --updated.
• The AskUserQuestion fallback prompt was removed and replaced with “same fallback as today,” which is ambiguous in a fresh session.

Methodology

None — no estimator/methodology changes in this PR.

Code Quality

• P1 | Impact: Conflicting requirements about the Delta Assessment when --updated is provided but no prior review exists; Step 1b says to skip the section while Step 5 says not to skip any section when the flag is present, leading to inconsistent output. | Concrete fix: Decide one behavior and make it explicit in both places; e.g., always include a Delta Assessment section with “Unavailable” placeholders, or update Step 5 to explicitly allow omitting the section when no prior review exists. (.claude/commands/review-plan.md:L39-L50, .claude/commands/review-plan.md:L168-L216)
• P2 | Impact: Treating --updated as a literal substring to remove can mis-parse legitimate file paths containing --updated, causing false “missing path” errors. | Concrete fix: Specify token-based parsing (split on whitespace, remove the exact --updated token), and clarify how to handle quoted paths. (.claude/commands/review-plan.md:L16-L19)

Performance

None.

Maintainability

• P2 | Impact: The fallback AskUserQuestion prompt is no longer specified; “same fallback as today” is ambiguous in a fresh session, leading to inconsistent prompts and harder future maintenance. | Concrete fix: Reintroduce the explicit AskUserQuestion template or provide the exact question/option text in the instructions. (.claude/commands/review-plan.md:L16-L19)

Tech Debt

None.

Security

None.

Documentation/Tests

None.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: c4ef7133922486dd767eeed21d9c3fa904c9a43b

---

Overall assessment: ⚠️ Needs changes

Executive summary:

• No estimator/methodology changes; docs/methodology/REGISTRY.md reviewed and not implicated.
• New argument parsing can break plan paths with spaces/quotes when --updated is used, leading to false “file not found.”
• Minor UX mismatch: argument hint suggests only post‑path flag placement despite allowing both orders.

Methodology

None. (Change is limited to .claude/commands/review-plan.md; no estimator/math/SE/assumption logic affected.)

Code Quality

• P2 (Impact: --updated parsing can corrupt quoted/space-containing paths, causing “plan file not found” and blocking review). Fix: parse $ARGUMENTS in a way that preserves quoted paths (e.g., remove --updated via a token/regex on the raw string, then trim) or explicitly state paths with spaces are unsupported. Location: .claude/commands/review-plan.md:L16-L19.

Performance

None.

Maintainability

None.

Tech Debt

None.

Security

None.

Documentation/Tests

• P3 (Impact: argument hint implies --updated only after path, which contradicts “before or after” guidance; minor user confusion). Fix: update hint to reflect both orders, e.g., argument-hint: "[--updated] <path-to-plan-file>" or "<path-to-plan-file> [--updated]" plus a short note. Location: .claude/commands/review-plan.md:L3.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: cbf53e9b14e9beae014468ef85812a42a7df233c

---

Overall assessment: ⚠️ Needs changes

Executive summary

• No estimator or methodology changes; REGISTRY/docstrings not applicable for this diff.
• New --updated parsing guidance can break paths with spaces or quotes; likely false “file not found.”
• Step 1 still says to read the raw $ARGUMENTS, which conflicts with the new parsing instructions when the flag is present.

Methodology

None. This PR only updates .claude/commands/review-plan.md and does not change estimators, math, weighting, SEs, identification assumptions, or defaults.

Code Quality

• P2 — Impact: whitespace splitting can corrupt paths with spaces/quotes, causing incorrect file resolution or unnecessary AskUserQuestion prompts. — Concrete fix: avoid split/join; instead remove a standalone --updated token from the raw argument string via a word-boundary regex and trim, or explicitly specify quoted-path handling and unquoting. (.claude/commands/review-plan.md:L16-L18)
• P3 — Impact: Step 1 can be interpreted to read the unparsed $ARGUMENTS, which would include --updated and fail when the flag is present. — Concrete fix: update Step 1 to say “Read the plan file at the parsed path after stripping --updated.” (.claude/commands/review-plan.md:L36-L38)

Performance

None.

Maintainability

None.

Tech Debt

None.

Security

None.

Documentation/Tests

None.