From e692f55cb20a346406bd060019273513a1b6dd60 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 06:37:13 +0000 Subject: [PATCH] feat(#413): add map-prd-review skill and write_prd_review artifact function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements issue #413: PRD/requirements-quality review before /map-plan. Changes: - Add `write_prd_review()` to `map_step_runner.py.jinja` (and rendered outputs): validates verdict, findings, blocking_questions, suggested_revisions; writes `.map//prd-review.{json,md}` and updates artifact_manifest stage `prd_review`; enforces `needs_user_decision` requires at least one blocking question; rejects extra/missing fields and invalid severities - Add `prd_review` to ARTIFACT_STAGES tuple so `_set_manifest_stage` accepts it - Add `PRD_REVIEW_SCHEMA` to `schemas.py` - Add `map-prd-review/SKILL.md.jinja` skill with 11 review dimensions, verdict table, step-by-step workflow, troubleshooting, and argument-hint - Register `map-prd-review` in `skill-rules.json.jinja` (type=manual, task) - 13 new tests covering happy paths (all 4 verdicts), error cases, and structural validation; update skill count sentinel from 20 → 21 Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01Jn1BLjiPwPBbnTTtZDRQgd --- .claude/skills/map-prd-review/SKILL.md | 152 ++++++++ .claude/skills/skill-rules.json | 25 ++ .map/scripts/map_step_runner.py | 302 ++++++++++++++++ src/mapify_cli/schemas.py | 67 ++++ .../templates/map/scripts/map_step_runner.py | 302 ++++++++++++++++ .../templates/skills/map-prd-review/SKILL.md | 152 ++++++++ .../templates/skills/skill-rules.json | 25 ++ .../map/scripts/map_step_runner.py.jinja | 302 ++++++++++++++++ .../skills/map-prd-review/SKILL.md.jinja | 152 ++++++++ .../skills/skill-rules.json.jinja | 25 ++ tests/test_map_step_runner.py | 325 ++++++++++++++++++ tests/test_skills_consistency.py | 10 +- 12 files changed, 1834 insertions(+), 5 deletions(-) create mode 100644 .claude/skills/map-prd-review/SKILL.md create mode 100644 src/mapify_cli/templates/skills/map-prd-review/SKILL.md create mode 100644 src/mapify_cli/templates_src/skills/map-prd-review/SKILL.md.jinja diff --git a/.claude/skills/map-prd-review/SKILL.md b/.claude/skills/map-prd-review/SKILL.md new file mode 100644 index 00000000..2bc94acd --- /dev/null +++ b/.claude/skills/map-prd-review/SKILL.md @@ -0,0 +1,152 @@ +--- +name: map-prd-review +description: | + PRD/requirements-quality review before /map-plan. Use when you have a product brief, PRD, or requirements note and need to verify its quality before starting planning. Reviews across 11 dimensions and returns a verdict: ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind. Writes durable artifacts to .map//prd-review.{json,md}. Do NOT use as a substitute for /map-plan, or on well-scoped engineering tasks that have no PRD. +effort: low +disable-model-invocation: true +argument-hint: "[path/to/prd.md or inline requirements text]" +--- +# MAP PRD / Requirements-Quality Review + +Reviews the quality of a supplied PRD, product brief, or requirements note **before** +`/map-plan` transforms it into a spec. A weak PRD causes MAP to invent product decisions, +produce unmeasurable acceptance criteria, and miss security or operational concerns. + +**This skill does NOT write code or start planning.** It produces a quality verdict and +blocking findings that the user resolves before calling `/map-plan`. + +## Input + +Provide one of: +- A path to a Markdown file: `/map-prd-review path/to/feature.md` +- Inline text pasted after the command. + +## Verdicts + +| Verdict | Meaning | Next step | +|---------|---------|-----------| +| `ready_for_plan` | Input is ready for `/map-plan` | Call `/map-plan` | +| `needs_prd_revision` | Fixable missing fields; PRD should be amended | Revise PRD, re-run | +| `needs_user_decision` | Product/design choices must be answered | Answer questions, re-run | +| `route_to_wayfind` | Input too foggy for a PRD review | Call `/map-wayfind` first | + +## Review Dimensions + +The reviewer inspects these dimensions and reports findings: + +1. **measurable_acceptance_criteria** — Are AC items pass/fail testable? +2. **user_job_clarity** — Is the target user and job-to-be-done explicit? +3. **explicit_out_of_scope** — Is what is NOT included stated? +4. **non_functional_requirements** — Are NFRs present when implied by domain/risk? +5. **data_model_lifecycle** — Is data shape, ownership, and lifecycle clear? +6. **ux_states_failure_states** — Are loading, empty, error, and edge UX states covered? +7. **security_trust_boundaries** — Are authentication, authorization, and trust surfaces stated? +8. **dependencies_integrations** — Are external systems and contracts named? +9. **contradictions_assumptions** — Are hidden assumptions or contradictions present? +10. **testability** — Can "done" be verified mechanically? +11. **migration_rollout** — Are migration/rollout/operational concerns addressed when relevant? + +## Effort and Parallelism Policy + +```yaml +thinking_policy: low/direct +parallel_tool_policy: sequential_by_default +``` + +- Run a single focused review; do not spawn sub-reviewers unless the PRD is multi-component. +- Do not write code, modify files outside `.map//`, or start planning. + +## Workflow + +### Step 1: Read the PRD + +Read the supplied file or text. If the argument is a file path that does not exist, report an error and stop. + +### Step 2: Review Against Dimensions + +For each of the 11 dimensions, decide: +- Does the PRD address this dimension adequately? +- If not, classify severity: `critical` (blocks planning), `major` (serious gap), `minor` (nice-to-have), `info` (note). +- Provide a concrete finding description and a suggested revision when applicable. + +### Step 3: Determine Verdict + +Apply this decision logic: + +1. Any `critical` finding → candidate for `needs_prd_revision` or `needs_user_decision`. +2. If the input is so vague/strategic that a PRD review cannot even identify the target user or feature → `route_to_wayfind`. +3. If there are concrete product/design choices that ONLY the human can make → `needs_user_decision` (must produce at least one blocking question). +4. If findings are fixable revisions (missing sections, vague AC) → `needs_prd_revision`. +5. No `critical` or `major` findings → `ready_for_plan`. + +### Step 4: Write Artifacts + +Call the step runner to persist results: + +``` +python3 .map/scripts/map_step_runner.py write_prd_review \ + --findings '' \ + --blocking-questions '' \ + --suggested-revisions '' \ + --summary "" \ + --prd-source "" +``` + +**findings JSON format:** +```json +[ + { + "dimension": "measurable_acceptance_criteria", + "severity": "major", + "description": "AC-2 says 'fast response' but does not specify a latency budget.", + "suggested_revision": "Define latency budget, e.g. p95 < 200ms under 100 RPS." + } +] +``` + +**blocking-questions JSON format (for needs_user_decision):** +```json +[ + { + "question": "Should the feature support unauthenticated users or require login?", + "category": "security_trust_boundaries" + } +] +``` + +**suggested-revisions JSON format (for needs_prd_revision):** +```json +[ + "Add an explicit out-of-scope section listing what this feature does NOT cover.", + "Replace 'should be fast' in AC-3 with a measurable latency target." +] +``` + +### Step 5: Report to User + +Show a summary of the verdict and key findings. If `ready_for_plan`, suggest calling `/map-plan`. + +## Artifacts Written + +- `.map//prd-review.json` — machine-readable verdict and findings +- `.map//prd-review.md` — human-readable review report + +## Examples + +``` +/map-prd-review docs/feature-brief.md +/map-prd-review We want to add a notification system that sends emails when orders ship. +``` + +## Troubleshooting + +- **File not found**: If you pass a path, confirm the file exists relative to the project root before invoking. +- **`route_to_wayfind` returned unexpectedly**: The input is too vague or strategic to review as a PRD. Call `/map-wayfind` first to resolve open decisions, then re-run. +- **Artifact not written**: Check that `.map//` is writable and `map_step_runner.py` is present in `.map/scripts/`. + +## Non-Goals + +- Do not replace `/map-plan`. This reviews the INPUT to planning, not the plan itself. +- Do not block tiny direct edits or well-scoped engineering tasks that have no PRD. +- Do not run multiple agent sub-reviews unless the PRD is unusually large. +- Do not store raw customer data or secrets in `.map/` artifacts. diff --git a/.claude/skills/skill-rules.json b/.claude/skills/skill-rules.json index 50afd1d6..13d1a1d8 100644 --- a/.claude/skills/skill-rules.json +++ b/.claude/skills/skill-rules.json @@ -427,6 +427,31 @@ "(find|identify|rank).*(hotspot|shallow|friction)" ] } + }, + "map-prd-review": { + "type": "manual", + "skillClass": "task", + "enforcement": "manual", + "priority": "medium", + "description": "PRD/requirements-quality review before /map-plan: reviews a product brief across 11 dimensions and returns ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind.", + "promptTriggers": { + "keywords": [ + "map-prd-review", + "prd review", + "requirements review", + "review requirements", + "review prd", + "product brief review", + "requirements quality", + "before map-plan" + ], + "intentPatterns": [ + "map-prd-review", + "(review|audit).*(prd|requirements|product brief|feature brief)", + "(prd|requirements).*(review|quality|ready)", + "before.*(map-plan|planning)" + ] + } } } } diff --git a/.map/scripts/map_step_runner.py b/.map/scripts/map_step_runner.py index 14669e21..61d4b048 100755 --- a/.map/scripts/map_step_runner.py +++ b/.map/scripts/map_step_runner.py @@ -261,6 +261,7 @@ def _effective_compression_threshold(policy: str, threshold: int) -> int | None: "context_usefulness", "wayfind_handoff", "review_verdict_ledger", + "prd_review", ) RUN_HEALTH_TERMINAL_STATUSES = { "pending", @@ -2971,6 +2972,266 @@ def write_implementer_readiness_review( return result +PRD_REVIEW_VERDICTS = frozenset( + {"ready_for_plan", "needs_prd_revision", "needs_user_decision", "route_to_wayfind"} +) +PRD_REVIEW_FINDING_SEVERITIES = frozenset({"critical", "major", "minor", "info"}) + + +def write_prd_review( + verdict: str, + findings_json: str = "[]", + blocking_questions_json: str = "[]", + suggested_revisions_json: str = "[]", + route_recommendation: str = "", + summary: str = "", + prd_source: str = "", + branch: str | None = None, +) -> dict[str, object]: + """Write PRD/requirements-quality review artifact and update artifact manifest. + + Writes: + .map//prd-review.json — machine-readable verdict + .map//prd-review.md — human-readable summary + + Verdict values: + ready_for_plan — input is ready for /map-plan + needs_prd_revision — PRD/brief should be amended before planning + needs_user_decision — specific product/design choices must be answered first + route_to_wayfind — input is too foggy; use /map-wayfind instead + """ + branch_name = branch or get_branch_name() + verdict = (verdict or "").strip().lower() + + if verdict not in PRD_REVIEW_VERDICTS: + return { + "status": "error", + "message": ( + f"Invalid verdict: {verdict!r}. " + f"Must be one of: {sorted(PRD_REVIEW_VERDICTS)}" + ), + } + + try: + parsed_findings = json.loads(findings_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid findings JSON: {exc}"} + if not isinstance(parsed_findings, list): + return {"status": "error", "message": "findings must be a JSON array"} + + try: + parsed_blocking_questions = json.loads(blocking_questions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid blocking_questions JSON: {exc}"} + if not isinstance(parsed_blocking_questions, list): + return {"status": "error", "message": "blocking_questions must be a JSON array"} + + try: + parsed_suggested_revisions = json.loads(suggested_revisions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid suggested_revisions JSON: {exc}"} + if not isinstance(parsed_suggested_revisions, list): + return {"status": "error", "message": "suggested_revisions must be a JSON array"} + + # Validate findings structure + allowed_finding_keys = {"dimension", "severity", "description", "suggested_revision"} + findings: list[dict[str, str]] = [] + for i, f in enumerate(parsed_findings): + if not isinstance(f, dict): + return {"status": "error", "message": f"findings[{i}] must be an object"} + extra_keys = set(f) - allowed_finding_keys + if extra_keys: + return { + "status": "error", + "message": f"findings[{i}] has unsupported fields: {sorted(extra_keys)}", + } + for req_field in ("dimension", "severity", "description"): + if req_field not in f: + return { + "status": "error", + "message": f"findings[{i}] missing required field {req_field!r}", + } + severity = f["severity"] + if severity not in PRD_REVIEW_FINDING_SEVERITIES: + return { + "status": "error", + "message": ( + f"findings[{i}].severity={severity!r} is not valid. " + f"Must be one of: {sorted(PRD_REVIEW_FINDING_SEVERITIES)}" + ), + } + normalized_finding: dict[str, str] = { + "dimension": f["dimension"], + "severity": severity, + "description": f["description"], + } + if "suggested_revision" in f: + suggested = f["suggested_revision"] + if not isinstance(suggested, str): + return { + "status": "error", + "message": f"findings[{i}].suggested_revision must be a string", + } + normalized_finding["suggested_revision"] = suggested + findings.append(normalized_finding) + + # Validate blocking questions structure + allowed_bq_keys = {"question", "category"} + blocking_questions: list[dict[str, str]] = [] + for i, q in enumerate(parsed_blocking_questions): + if not isinstance(q, dict): + return {"status": "error", "message": f"blocking_questions[{i}] must be an object"} + extra_keys = set(q) - allowed_bq_keys + if extra_keys: + return { + "status": "error", + "message": ( + f"blocking_questions[{i}] has unsupported fields: {sorted(extra_keys)}" + ), + } + for req_field in ("question", "category"): + if req_field not in q: + return { + "status": "error", + "message": f"blocking_questions[{i}] missing required field {req_field!r}", + } + blocking_questions.append({"question": q["question"], "category": q["category"]}) + + # Validate suggested revisions + suggested_revisions: list[str] = [] + for i, rev in enumerate(parsed_suggested_revisions): + if not isinstance(rev, str): + return { + "status": "error", + "message": f"suggested_revisions[{i}] must be a string", + } + suggested_revisions.append(rev) + + if verdict == "needs_user_decision" and not blocking_questions: + return { + "status": "error", + "message": ( + "blocking_questions must be non-empty when verdict is " + "'needs_user_decision'. Provide at least one question that " + "must be answered before planning can proceed." + ), + } + + branch_dir = get_branch_dir(branch_name) + branch_dir.mkdir(parents=True, exist_ok=True) + + payload: dict[str, object] = { + "schema_version": "1.0", + "branch": branch_name, + "generated_at": _utc_timestamp(), + "verdict": verdict, + "findings": findings, + "blocking_questions": blocking_questions, + "suggested_revisions": suggested_revisions, + "summary": summary or "No summary provided.", + } + if prd_source.strip(): + payload["prd_source"] = prd_source.strip() + if route_recommendation.strip(): + payload["route_recommendation"] = route_recommendation.strip() + + json_path = branch_dir / "prd-review.json" + _write_json_file(json_path, payload) + + # Human-readable Markdown report + verdict_label = { + "ready_for_plan": "READY FOR PLAN", + "needs_prd_revision": "NEEDS PRD REVISION", + "needs_user_decision": "NEEDS USER DECISION", + "route_to_wayfind": "ROUTE TO WAYFIND", + }.get(verdict, verdict.upper()) + + md_lines = [ + "# PRD / Requirements-Quality Review", + "", + f"**Verdict:** {verdict_label}", + f"**Branch:** `{branch_name}`", + f"**Generated:** {payload['generated_at']}", + ] + if prd_source.strip(): + md_lines.append(f"**PRD Source:** `{prd_source.strip()}`") + md_lines += ["", "## Summary", "", payload["summary"], ""] # type: ignore[arg-type] + + if findings: + md_lines += ["## Findings", ""] + for finding in findings: + sev = finding["severity"].upper() + dim = finding["dimension"] + desc = finding["description"] + md_lines.append(f"- **[{sev}]** `{dim}`: {desc}") + if "suggested_revision" in finding: + md_lines.append(f" - *Suggested:* {finding['suggested_revision']}") + md_lines.append("") + + if blocking_questions: + md_lines += ["## Blocking Questions", ""] + for i, q in enumerate(blocking_questions, 1): + cat = q.get("category", "unspecified") + md_lines.append(f"{i}. **[{cat}]** {q['question']}") + md_lines.append("") + + if suggested_revisions: + md_lines += ["## Suggested Revisions", ""] + for rev in suggested_revisions: + md_lines.append(f"- {rev}") + md_lines.append("") + + if route_recommendation.strip(): + md_lines += [ + "## Route Recommendation", + "", + route_recommendation.strip(), + "", + ] + + md_path = branch_dir / "prd-review.md" + md_path.write_text("\n".join(md_lines), encoding="utf-8") + + manifest = load_artifact_manifest(branch_name) + _set_manifest_stage( + manifest, + "prd_review", + "ready", + artifacts=[ + _artifact_ref(json_path, "prd-review"), + _artifact_ref(md_path, "prd-review-report"), + ], + metadata={ + "verdict": verdict, + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + }, + ) + manifest_result = save_artifact_manifest(manifest, branch_name) + + verdict_messages = { + "ready_for_plan": "PRD is ready for /map-plan.", + "needs_prd_revision": "PRD must be revised before planning. See suggested_revisions.", + "needs_user_decision": "Blocking product decisions must be answered before planning.", + "route_to_wayfind": "Input is too foggy for PRD review; use /map-wayfind instead.", + } + + result: dict[str, object] = { + "status": "success", + "verdict": verdict, + "proceed": verdict == "ready_for_plan", + "json_path": str(json_path), + "md_path": str(md_path), + "manifest_path": manifest_result["path"], + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + "message": verdict_messages.get(verdict, verdict), + } + return result + + def record_plan_artifacts(branch: str | None = None) -> dict[str, object]: """Persist spec/plan artifact presence into artifact_manifest.json.""" branch_name = branch or get_branch_name() @@ -21401,6 +21662,47 @@ def _irr_flag(name: str, default: str = "") -> str: if result.get("status") == "error": sys.exit(1) + elif func_name == "write_prd_review" and len(sys.argv) >= 3: + # CLI: write_prd_review + # [--findings ''] + # [--blocking-questions ''] + # [--suggested-revisions ''] + # [--route-recommendation "..."] + # [--summary "..."] + # [--prd-source "path/or/label"] + # [--branch ] + # + # verdict must be one of: + # ready_for_plan needs_prd_revision needs_user_decision route_to_wayfind + # + # findings JSON format: + # '[{"dimension":"measurable_acceptance_criteria","severity":"major","description":"..."}]' + # blocking-questions JSON format: + # '[{"question":"...","category":"product_decision"}]' + # suggested-revisions JSON format: + # '["Revise AC-1 to specify latency budget","Add out-of-scope section"]' + def _prr_flag(name: str, default: str = "") -> str: + flag = f"--{name}" + if flag in sys.argv: + idx = sys.argv.index(flag) + if idx + 1 < len(sys.argv): + return sys.argv[idx + 1] + return default + + result = write_prd_review( + sys.argv[2], + findings_json=_prr_flag("findings", "[]"), + blocking_questions_json=_prr_flag("blocking-questions", "[]"), + suggested_revisions_json=_prr_flag("suggested-revisions", "[]"), + route_recommendation=_prr_flag("route-recommendation", ""), + summary=_prr_flag("summary", ""), + prd_source=_prr_flag("prd-source", ""), + branch=_prr_flag("branch") or None, + ) + print(json.dumps(result, indent=2, ensure_ascii=True)) + if result.get("status") == "error": + sys.exit(1) + elif func_name == "write_review_verdict_ledger": # CLI: write_review_verdict_ledger # [--monitor-json ''] diff --git a/src/mapify_cli/schemas.py b/src/mapify_cli/schemas.py index a3e93c46..bbb1dd23 100644 --- a/src/mapify_cli/schemas.py +++ b/src/mapify_cli/schemas.py @@ -2055,3 +2055,70 @@ def load_and_validate( ], "additionalProperties": True, } + +PRD_REVIEW_SCHEMA: dict[str, Any] = { + "$id": "prd-review.json", + "type": "object", + "description": "PRD/requirements-quality review artifact written by write_prd_review.", + "properties": { + "schema_version": {"type": "string"}, + "branch": {"type": "string"}, + "generated_at": {"type": "string", "description": "ISO-8601 UTC timestamp"}, + "prd_source": {"type": "string", "description": "Path or label of the reviewed PRD"}, + "verdict": { + "type": "string", + "enum": [ + "ready_for_plan", + "needs_prd_revision", + "needs_user_decision", + "route_to_wayfind", + ], + }, + "findings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "dimension": {"type": "string"}, + "severity": { + "type": "string", + "enum": ["critical", "major", "minor", "info"], + }, + "description": {"type": "string"}, + "suggested_revision": {"type": "string"}, + }, + "required": ["dimension", "severity", "description"], + "additionalProperties": False, + }, + }, + "blocking_questions": { + "type": "array", + "items": { + "type": "object", + "properties": { + "question": {"type": "string"}, + "category": {"type": "string"}, + }, + "required": ["question", "category"], + "additionalProperties": False, + }, + }, + "suggested_revisions": { + "type": "array", + "items": {"type": "string"}, + }, + "route_recommendation": {"type": "string"}, + "summary": {"type": "string"}, + }, + "required": [ + "schema_version", + "branch", + "generated_at", + "verdict", + "findings", + "blocking_questions", + "suggested_revisions", + "summary", + ], + "additionalProperties": True, +} diff --git a/src/mapify_cli/templates/map/scripts/map_step_runner.py b/src/mapify_cli/templates/map/scripts/map_step_runner.py index 14669e21..61d4b048 100755 --- a/src/mapify_cli/templates/map/scripts/map_step_runner.py +++ b/src/mapify_cli/templates/map/scripts/map_step_runner.py @@ -261,6 +261,7 @@ def _effective_compression_threshold(policy: str, threshold: int) -> int | None: "context_usefulness", "wayfind_handoff", "review_verdict_ledger", + "prd_review", ) RUN_HEALTH_TERMINAL_STATUSES = { "pending", @@ -2971,6 +2972,266 @@ def write_implementer_readiness_review( return result +PRD_REVIEW_VERDICTS = frozenset( + {"ready_for_plan", "needs_prd_revision", "needs_user_decision", "route_to_wayfind"} +) +PRD_REVIEW_FINDING_SEVERITIES = frozenset({"critical", "major", "minor", "info"}) + + +def write_prd_review( + verdict: str, + findings_json: str = "[]", + blocking_questions_json: str = "[]", + suggested_revisions_json: str = "[]", + route_recommendation: str = "", + summary: str = "", + prd_source: str = "", + branch: str | None = None, +) -> dict[str, object]: + """Write PRD/requirements-quality review artifact and update artifact manifest. + + Writes: + .map//prd-review.json — machine-readable verdict + .map//prd-review.md — human-readable summary + + Verdict values: + ready_for_plan — input is ready for /map-plan + needs_prd_revision — PRD/brief should be amended before planning + needs_user_decision — specific product/design choices must be answered first + route_to_wayfind — input is too foggy; use /map-wayfind instead + """ + branch_name = branch or get_branch_name() + verdict = (verdict or "").strip().lower() + + if verdict not in PRD_REVIEW_VERDICTS: + return { + "status": "error", + "message": ( + f"Invalid verdict: {verdict!r}. " + f"Must be one of: {sorted(PRD_REVIEW_VERDICTS)}" + ), + } + + try: + parsed_findings = json.loads(findings_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid findings JSON: {exc}"} + if not isinstance(parsed_findings, list): + return {"status": "error", "message": "findings must be a JSON array"} + + try: + parsed_blocking_questions = json.loads(blocking_questions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid blocking_questions JSON: {exc}"} + if not isinstance(parsed_blocking_questions, list): + return {"status": "error", "message": "blocking_questions must be a JSON array"} + + try: + parsed_suggested_revisions = json.loads(suggested_revisions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid suggested_revisions JSON: {exc}"} + if not isinstance(parsed_suggested_revisions, list): + return {"status": "error", "message": "suggested_revisions must be a JSON array"} + + # Validate findings structure + allowed_finding_keys = {"dimension", "severity", "description", "suggested_revision"} + findings: list[dict[str, str]] = [] + for i, f in enumerate(parsed_findings): + if not isinstance(f, dict): + return {"status": "error", "message": f"findings[{i}] must be an object"} + extra_keys = set(f) - allowed_finding_keys + if extra_keys: + return { + "status": "error", + "message": f"findings[{i}] has unsupported fields: {sorted(extra_keys)}", + } + for req_field in ("dimension", "severity", "description"): + if req_field not in f: + return { + "status": "error", + "message": f"findings[{i}] missing required field {req_field!r}", + } + severity = f["severity"] + if severity not in PRD_REVIEW_FINDING_SEVERITIES: + return { + "status": "error", + "message": ( + f"findings[{i}].severity={severity!r} is not valid. " + f"Must be one of: {sorted(PRD_REVIEW_FINDING_SEVERITIES)}" + ), + } + normalized_finding: dict[str, str] = { + "dimension": f["dimension"], + "severity": severity, + "description": f["description"], + } + if "suggested_revision" in f: + suggested = f["suggested_revision"] + if not isinstance(suggested, str): + return { + "status": "error", + "message": f"findings[{i}].suggested_revision must be a string", + } + normalized_finding["suggested_revision"] = suggested + findings.append(normalized_finding) + + # Validate blocking questions structure + allowed_bq_keys = {"question", "category"} + blocking_questions: list[dict[str, str]] = [] + for i, q in enumerate(parsed_blocking_questions): + if not isinstance(q, dict): + return {"status": "error", "message": f"blocking_questions[{i}] must be an object"} + extra_keys = set(q) - allowed_bq_keys + if extra_keys: + return { + "status": "error", + "message": ( + f"blocking_questions[{i}] has unsupported fields: {sorted(extra_keys)}" + ), + } + for req_field in ("question", "category"): + if req_field not in q: + return { + "status": "error", + "message": f"blocking_questions[{i}] missing required field {req_field!r}", + } + blocking_questions.append({"question": q["question"], "category": q["category"]}) + + # Validate suggested revisions + suggested_revisions: list[str] = [] + for i, rev in enumerate(parsed_suggested_revisions): + if not isinstance(rev, str): + return { + "status": "error", + "message": f"suggested_revisions[{i}] must be a string", + } + suggested_revisions.append(rev) + + if verdict == "needs_user_decision" and not blocking_questions: + return { + "status": "error", + "message": ( + "blocking_questions must be non-empty when verdict is " + "'needs_user_decision'. Provide at least one question that " + "must be answered before planning can proceed." + ), + } + + branch_dir = get_branch_dir(branch_name) + branch_dir.mkdir(parents=True, exist_ok=True) + + payload: dict[str, object] = { + "schema_version": "1.0", + "branch": branch_name, + "generated_at": _utc_timestamp(), + "verdict": verdict, + "findings": findings, + "blocking_questions": blocking_questions, + "suggested_revisions": suggested_revisions, + "summary": summary or "No summary provided.", + } + if prd_source.strip(): + payload["prd_source"] = prd_source.strip() + if route_recommendation.strip(): + payload["route_recommendation"] = route_recommendation.strip() + + json_path = branch_dir / "prd-review.json" + _write_json_file(json_path, payload) + + # Human-readable Markdown report + verdict_label = { + "ready_for_plan": "READY FOR PLAN", + "needs_prd_revision": "NEEDS PRD REVISION", + "needs_user_decision": "NEEDS USER DECISION", + "route_to_wayfind": "ROUTE TO WAYFIND", + }.get(verdict, verdict.upper()) + + md_lines = [ + "# PRD / Requirements-Quality Review", + "", + f"**Verdict:** {verdict_label}", + f"**Branch:** `{branch_name}`", + f"**Generated:** {payload['generated_at']}", + ] + if prd_source.strip(): + md_lines.append(f"**PRD Source:** `{prd_source.strip()}`") + md_lines += ["", "## Summary", "", payload["summary"], ""] # type: ignore[arg-type] + + if findings: + md_lines += ["## Findings", ""] + for finding in findings: + sev = finding["severity"].upper() + dim = finding["dimension"] + desc = finding["description"] + md_lines.append(f"- **[{sev}]** `{dim}`: {desc}") + if "suggested_revision" in finding: + md_lines.append(f" - *Suggested:* {finding['suggested_revision']}") + md_lines.append("") + + if blocking_questions: + md_lines += ["## Blocking Questions", ""] + for i, q in enumerate(blocking_questions, 1): + cat = q.get("category", "unspecified") + md_lines.append(f"{i}. **[{cat}]** {q['question']}") + md_lines.append("") + + if suggested_revisions: + md_lines += ["## Suggested Revisions", ""] + for rev in suggested_revisions: + md_lines.append(f"- {rev}") + md_lines.append("") + + if route_recommendation.strip(): + md_lines += [ + "## Route Recommendation", + "", + route_recommendation.strip(), + "", + ] + + md_path = branch_dir / "prd-review.md" + md_path.write_text("\n".join(md_lines), encoding="utf-8") + + manifest = load_artifact_manifest(branch_name) + _set_manifest_stage( + manifest, + "prd_review", + "ready", + artifacts=[ + _artifact_ref(json_path, "prd-review"), + _artifact_ref(md_path, "prd-review-report"), + ], + metadata={ + "verdict": verdict, + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + }, + ) + manifest_result = save_artifact_manifest(manifest, branch_name) + + verdict_messages = { + "ready_for_plan": "PRD is ready for /map-plan.", + "needs_prd_revision": "PRD must be revised before planning. See suggested_revisions.", + "needs_user_decision": "Blocking product decisions must be answered before planning.", + "route_to_wayfind": "Input is too foggy for PRD review; use /map-wayfind instead.", + } + + result: dict[str, object] = { + "status": "success", + "verdict": verdict, + "proceed": verdict == "ready_for_plan", + "json_path": str(json_path), + "md_path": str(md_path), + "manifest_path": manifest_result["path"], + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + "message": verdict_messages.get(verdict, verdict), + } + return result + + def record_plan_artifacts(branch: str | None = None) -> dict[str, object]: """Persist spec/plan artifact presence into artifact_manifest.json.""" branch_name = branch or get_branch_name() @@ -21401,6 +21662,47 @@ def _irr_flag(name: str, default: str = "") -> str: if result.get("status") == "error": sys.exit(1) + elif func_name == "write_prd_review" and len(sys.argv) >= 3: + # CLI: write_prd_review + # [--findings ''] + # [--blocking-questions ''] + # [--suggested-revisions ''] + # [--route-recommendation "..."] + # [--summary "..."] + # [--prd-source "path/or/label"] + # [--branch ] + # + # verdict must be one of: + # ready_for_plan needs_prd_revision needs_user_decision route_to_wayfind + # + # findings JSON format: + # '[{"dimension":"measurable_acceptance_criteria","severity":"major","description":"..."}]' + # blocking-questions JSON format: + # '[{"question":"...","category":"product_decision"}]' + # suggested-revisions JSON format: + # '["Revise AC-1 to specify latency budget","Add out-of-scope section"]' + def _prr_flag(name: str, default: str = "") -> str: + flag = f"--{name}" + if flag in sys.argv: + idx = sys.argv.index(flag) + if idx + 1 < len(sys.argv): + return sys.argv[idx + 1] + return default + + result = write_prd_review( + sys.argv[2], + findings_json=_prr_flag("findings", "[]"), + blocking_questions_json=_prr_flag("blocking-questions", "[]"), + suggested_revisions_json=_prr_flag("suggested-revisions", "[]"), + route_recommendation=_prr_flag("route-recommendation", ""), + summary=_prr_flag("summary", ""), + prd_source=_prr_flag("prd-source", ""), + branch=_prr_flag("branch") or None, + ) + print(json.dumps(result, indent=2, ensure_ascii=True)) + if result.get("status") == "error": + sys.exit(1) + elif func_name == "write_review_verdict_ledger": # CLI: write_review_verdict_ledger # [--monitor-json ''] diff --git a/src/mapify_cli/templates/skills/map-prd-review/SKILL.md b/src/mapify_cli/templates/skills/map-prd-review/SKILL.md new file mode 100644 index 00000000..2bc94acd --- /dev/null +++ b/src/mapify_cli/templates/skills/map-prd-review/SKILL.md @@ -0,0 +1,152 @@ +--- +name: map-prd-review +description: | + PRD/requirements-quality review before /map-plan. Use when you have a product brief, PRD, or requirements note and need to verify its quality before starting planning. Reviews across 11 dimensions and returns a verdict: ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind. Writes durable artifacts to .map//prd-review.{json,md}. Do NOT use as a substitute for /map-plan, or on well-scoped engineering tasks that have no PRD. +effort: low +disable-model-invocation: true +argument-hint: "[path/to/prd.md or inline requirements text]" +--- +# MAP PRD / Requirements-Quality Review + +Reviews the quality of a supplied PRD, product brief, or requirements note **before** +`/map-plan` transforms it into a spec. A weak PRD causes MAP to invent product decisions, +produce unmeasurable acceptance criteria, and miss security or operational concerns. + +**This skill does NOT write code or start planning.** It produces a quality verdict and +blocking findings that the user resolves before calling `/map-plan`. + +## Input + +Provide one of: +- A path to a Markdown file: `/map-prd-review path/to/feature.md` +- Inline text pasted after the command. + +## Verdicts + +| Verdict | Meaning | Next step | +|---------|---------|-----------| +| `ready_for_plan` | Input is ready for `/map-plan` | Call `/map-plan` | +| `needs_prd_revision` | Fixable missing fields; PRD should be amended | Revise PRD, re-run | +| `needs_user_decision` | Product/design choices must be answered | Answer questions, re-run | +| `route_to_wayfind` | Input too foggy for a PRD review | Call `/map-wayfind` first | + +## Review Dimensions + +The reviewer inspects these dimensions and reports findings: + +1. **measurable_acceptance_criteria** — Are AC items pass/fail testable? +2. **user_job_clarity** — Is the target user and job-to-be-done explicit? +3. **explicit_out_of_scope** — Is what is NOT included stated? +4. **non_functional_requirements** — Are NFRs present when implied by domain/risk? +5. **data_model_lifecycle** — Is data shape, ownership, and lifecycle clear? +6. **ux_states_failure_states** — Are loading, empty, error, and edge UX states covered? +7. **security_trust_boundaries** — Are authentication, authorization, and trust surfaces stated? +8. **dependencies_integrations** — Are external systems and contracts named? +9. **contradictions_assumptions** — Are hidden assumptions or contradictions present? +10. **testability** — Can "done" be verified mechanically? +11. **migration_rollout** — Are migration/rollout/operational concerns addressed when relevant? + +## Effort and Parallelism Policy + +```yaml +thinking_policy: low/direct +parallel_tool_policy: sequential_by_default +``` + +- Run a single focused review; do not spawn sub-reviewers unless the PRD is multi-component. +- Do not write code, modify files outside `.map//`, or start planning. + +## Workflow + +### Step 1: Read the PRD + +Read the supplied file or text. If the argument is a file path that does not exist, report an error and stop. + +### Step 2: Review Against Dimensions + +For each of the 11 dimensions, decide: +- Does the PRD address this dimension adequately? +- If not, classify severity: `critical` (blocks planning), `major` (serious gap), `minor` (nice-to-have), `info` (note). +- Provide a concrete finding description and a suggested revision when applicable. + +### Step 3: Determine Verdict + +Apply this decision logic: + +1. Any `critical` finding → candidate for `needs_prd_revision` or `needs_user_decision`. +2. If the input is so vague/strategic that a PRD review cannot even identify the target user or feature → `route_to_wayfind`. +3. If there are concrete product/design choices that ONLY the human can make → `needs_user_decision` (must produce at least one blocking question). +4. If findings are fixable revisions (missing sections, vague AC) → `needs_prd_revision`. +5. No `critical` or `major` findings → `ready_for_plan`. + +### Step 4: Write Artifacts + +Call the step runner to persist results: + +``` +python3 .map/scripts/map_step_runner.py write_prd_review \ + --findings '' \ + --blocking-questions '' \ + --suggested-revisions '' \ + --summary "" \ + --prd-source "" +``` + +**findings JSON format:** +```json +[ + { + "dimension": "measurable_acceptance_criteria", + "severity": "major", + "description": "AC-2 says 'fast response' but does not specify a latency budget.", + "suggested_revision": "Define latency budget, e.g. p95 < 200ms under 100 RPS." + } +] +``` + +**blocking-questions JSON format (for needs_user_decision):** +```json +[ + { + "question": "Should the feature support unauthenticated users or require login?", + "category": "security_trust_boundaries" + } +] +``` + +**suggested-revisions JSON format (for needs_prd_revision):** +```json +[ + "Add an explicit out-of-scope section listing what this feature does NOT cover.", + "Replace 'should be fast' in AC-3 with a measurable latency target." +] +``` + +### Step 5: Report to User + +Show a summary of the verdict and key findings. If `ready_for_plan`, suggest calling `/map-plan`. + +## Artifacts Written + +- `.map//prd-review.json` — machine-readable verdict and findings +- `.map//prd-review.md` — human-readable review report + +## Examples + +``` +/map-prd-review docs/feature-brief.md +/map-prd-review We want to add a notification system that sends emails when orders ship. +``` + +## Troubleshooting + +- **File not found**: If you pass a path, confirm the file exists relative to the project root before invoking. +- **`route_to_wayfind` returned unexpectedly**: The input is too vague or strategic to review as a PRD. Call `/map-wayfind` first to resolve open decisions, then re-run. +- **Artifact not written**: Check that `.map//` is writable and `map_step_runner.py` is present in `.map/scripts/`. + +## Non-Goals + +- Do not replace `/map-plan`. This reviews the INPUT to planning, not the plan itself. +- Do not block tiny direct edits or well-scoped engineering tasks that have no PRD. +- Do not run multiple agent sub-reviews unless the PRD is unusually large. +- Do not store raw customer data or secrets in `.map/` artifacts. diff --git a/src/mapify_cli/templates/skills/skill-rules.json b/src/mapify_cli/templates/skills/skill-rules.json index 50afd1d6..13d1a1d8 100644 --- a/src/mapify_cli/templates/skills/skill-rules.json +++ b/src/mapify_cli/templates/skills/skill-rules.json @@ -427,6 +427,31 @@ "(find|identify|rank).*(hotspot|shallow|friction)" ] } + }, + "map-prd-review": { + "type": "manual", + "skillClass": "task", + "enforcement": "manual", + "priority": "medium", + "description": "PRD/requirements-quality review before /map-plan: reviews a product brief across 11 dimensions and returns ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind.", + "promptTriggers": { + "keywords": [ + "map-prd-review", + "prd review", + "requirements review", + "review requirements", + "review prd", + "product brief review", + "requirements quality", + "before map-plan" + ], + "intentPatterns": [ + "map-prd-review", + "(review|audit).*(prd|requirements|product brief|feature brief)", + "(prd|requirements).*(review|quality|ready)", + "before.*(map-plan|planning)" + ] + } } } } diff --git a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja index 14669e21..61d4b048 100755 --- a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja +++ b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja @@ -261,6 +261,7 @@ ARTIFACT_STAGE_NAMES = ( "context_usefulness", "wayfind_handoff", "review_verdict_ledger", + "prd_review", ) RUN_HEALTH_TERMINAL_STATUSES = { "pending", @@ -2971,6 +2972,266 @@ def write_implementer_readiness_review( return result +PRD_REVIEW_VERDICTS = frozenset( + {"ready_for_plan", "needs_prd_revision", "needs_user_decision", "route_to_wayfind"} +) +PRD_REVIEW_FINDING_SEVERITIES = frozenset({"critical", "major", "minor", "info"}) + + +def write_prd_review( + verdict: str, + findings_json: str = "[]", + blocking_questions_json: str = "[]", + suggested_revisions_json: str = "[]", + route_recommendation: str = "", + summary: str = "", + prd_source: str = "", + branch: str | None = None, +) -> dict[str, object]: + """Write PRD/requirements-quality review artifact and update artifact manifest. + + Writes: + .map//prd-review.json — machine-readable verdict + .map//prd-review.md — human-readable summary + + Verdict values: + ready_for_plan — input is ready for /map-plan + needs_prd_revision — PRD/brief should be amended before planning + needs_user_decision — specific product/design choices must be answered first + route_to_wayfind — input is too foggy; use /map-wayfind instead + """ + branch_name = branch or get_branch_name() + verdict = (verdict or "").strip().lower() + + if verdict not in PRD_REVIEW_VERDICTS: + return { + "status": "error", + "message": ( + f"Invalid verdict: {verdict!r}. " + f"Must be one of: {sorted(PRD_REVIEW_VERDICTS)}" + ), + } + + try: + parsed_findings = json.loads(findings_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid findings JSON: {exc}"} + if not isinstance(parsed_findings, list): + return {"status": "error", "message": "findings must be a JSON array"} + + try: + parsed_blocking_questions = json.loads(blocking_questions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid blocking_questions JSON: {exc}"} + if not isinstance(parsed_blocking_questions, list): + return {"status": "error", "message": "blocking_questions must be a JSON array"} + + try: + parsed_suggested_revisions = json.loads(suggested_revisions_json or "[]") + except json.JSONDecodeError as exc: + return {"status": "error", "message": f"Invalid suggested_revisions JSON: {exc}"} + if not isinstance(parsed_suggested_revisions, list): + return {"status": "error", "message": "suggested_revisions must be a JSON array"} + + # Validate findings structure + allowed_finding_keys = {"dimension", "severity", "description", "suggested_revision"} + findings: list[dict[str, str]] = [] + for i, f in enumerate(parsed_findings): + if not isinstance(f, dict): + return {"status": "error", "message": f"findings[{i}] must be an object"} + extra_keys = set(f) - allowed_finding_keys + if extra_keys: + return { + "status": "error", + "message": f"findings[{i}] has unsupported fields: {sorted(extra_keys)}", + } + for req_field in ("dimension", "severity", "description"): + if req_field not in f: + return { + "status": "error", + "message": f"findings[{i}] missing required field {req_field!r}", + } + severity = f["severity"] + if severity not in PRD_REVIEW_FINDING_SEVERITIES: + return { + "status": "error", + "message": ( + f"findings[{i}].severity={severity!r} is not valid. " + f"Must be one of: {sorted(PRD_REVIEW_FINDING_SEVERITIES)}" + ), + } + normalized_finding: dict[str, str] = { + "dimension": f["dimension"], + "severity": severity, + "description": f["description"], + } + if "suggested_revision" in f: + suggested = f["suggested_revision"] + if not isinstance(suggested, str): + return { + "status": "error", + "message": f"findings[{i}].suggested_revision must be a string", + } + normalized_finding["suggested_revision"] = suggested + findings.append(normalized_finding) + + # Validate blocking questions structure + allowed_bq_keys = {"question", "category"} + blocking_questions: list[dict[str, str]] = [] + for i, q in enumerate(parsed_blocking_questions): + if not isinstance(q, dict): + return {"status": "error", "message": f"blocking_questions[{i}] must be an object"} + extra_keys = set(q) - allowed_bq_keys + if extra_keys: + return { + "status": "error", + "message": ( + f"blocking_questions[{i}] has unsupported fields: {sorted(extra_keys)}" + ), + } + for req_field in ("question", "category"): + if req_field not in q: + return { + "status": "error", + "message": f"blocking_questions[{i}] missing required field {req_field!r}", + } + blocking_questions.append({"question": q["question"], "category": q["category"]}) + + # Validate suggested revisions + suggested_revisions: list[str] = [] + for i, rev in enumerate(parsed_suggested_revisions): + if not isinstance(rev, str): + return { + "status": "error", + "message": f"suggested_revisions[{i}] must be a string", + } + suggested_revisions.append(rev) + + if verdict == "needs_user_decision" and not blocking_questions: + return { + "status": "error", + "message": ( + "blocking_questions must be non-empty when verdict is " + "'needs_user_decision'. Provide at least one question that " + "must be answered before planning can proceed." + ), + } + + branch_dir = get_branch_dir(branch_name) + branch_dir.mkdir(parents=True, exist_ok=True) + + payload: dict[str, object] = { + "schema_version": "1.0", + "branch": branch_name, + "generated_at": _utc_timestamp(), + "verdict": verdict, + "findings": findings, + "blocking_questions": blocking_questions, + "suggested_revisions": suggested_revisions, + "summary": summary or "No summary provided.", + } + if prd_source.strip(): + payload["prd_source"] = prd_source.strip() + if route_recommendation.strip(): + payload["route_recommendation"] = route_recommendation.strip() + + json_path = branch_dir / "prd-review.json" + _write_json_file(json_path, payload) + + # Human-readable Markdown report + verdict_label = { + "ready_for_plan": "READY FOR PLAN", + "needs_prd_revision": "NEEDS PRD REVISION", + "needs_user_decision": "NEEDS USER DECISION", + "route_to_wayfind": "ROUTE TO WAYFIND", + }.get(verdict, verdict.upper()) + + md_lines = [ + "# PRD / Requirements-Quality Review", + "", + f"**Verdict:** {verdict_label}", + f"**Branch:** `{branch_name}`", + f"**Generated:** {payload['generated_at']}", + ] + if prd_source.strip(): + md_lines.append(f"**PRD Source:** `{prd_source.strip()}`") + md_lines += ["", "## Summary", "", payload["summary"], ""] # type: ignore[arg-type] + + if findings: + md_lines += ["## Findings", ""] + for finding in findings: + sev = finding["severity"].upper() + dim = finding["dimension"] + desc = finding["description"] + md_lines.append(f"- **[{sev}]** `{dim}`: {desc}") + if "suggested_revision" in finding: + md_lines.append(f" - *Suggested:* {finding['suggested_revision']}") + md_lines.append("") + + if blocking_questions: + md_lines += ["## Blocking Questions", ""] + for i, q in enumerate(blocking_questions, 1): + cat = q.get("category", "unspecified") + md_lines.append(f"{i}. **[{cat}]** {q['question']}") + md_lines.append("") + + if suggested_revisions: + md_lines += ["## Suggested Revisions", ""] + for rev in suggested_revisions: + md_lines.append(f"- {rev}") + md_lines.append("") + + if route_recommendation.strip(): + md_lines += [ + "## Route Recommendation", + "", + route_recommendation.strip(), + "", + ] + + md_path = branch_dir / "prd-review.md" + md_path.write_text("\n".join(md_lines), encoding="utf-8") + + manifest = load_artifact_manifest(branch_name) + _set_manifest_stage( + manifest, + "prd_review", + "ready", + artifacts=[ + _artifact_ref(json_path, "prd-review"), + _artifact_ref(md_path, "prd-review-report"), + ], + metadata={ + "verdict": verdict, + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + }, + ) + manifest_result = save_artifact_manifest(manifest, branch_name) + + verdict_messages = { + "ready_for_plan": "PRD is ready for /map-plan.", + "needs_prd_revision": "PRD must be revised before planning. See suggested_revisions.", + "needs_user_decision": "Blocking product decisions must be answered before planning.", + "route_to_wayfind": "Input is too foggy for PRD review; use /map-wayfind instead.", + } + + result: dict[str, object] = { + "status": "success", + "verdict": verdict, + "proceed": verdict == "ready_for_plan", + "json_path": str(json_path), + "md_path": str(md_path), + "manifest_path": manifest_result["path"], + "findings_count": len(findings), + "blocking_questions_count": len(blocking_questions), + "suggested_revisions_count": len(suggested_revisions), + "message": verdict_messages.get(verdict, verdict), + } + return result + + def record_plan_artifacts(branch: str | None = None) -> dict[str, object]: """Persist spec/plan artifact presence into artifact_manifest.json.""" branch_name = branch or get_branch_name() @@ -21401,6 +21662,47 @@ if __name__ == "__main__": if result.get("status") == "error": sys.exit(1) + elif func_name == "write_prd_review" and len(sys.argv) >= 3: + # CLI: write_prd_review + # [--findings ''] + # [--blocking-questions ''] + # [--suggested-revisions ''] + # [--route-recommendation "..."] + # [--summary "..."] + # [--prd-source "path/or/label"] + # [--branch ] + # + # verdict must be one of: + # ready_for_plan needs_prd_revision needs_user_decision route_to_wayfind + # + # findings JSON format: + # '[{"dimension":"measurable_acceptance_criteria","severity":"major","description":"..."}]' + # blocking-questions JSON format: + # '[{"question":"...","category":"product_decision"}]' + # suggested-revisions JSON format: + # '["Revise AC-1 to specify latency budget","Add out-of-scope section"]' + def _prr_flag(name: str, default: str = "") -> str: + flag = f"--{name}" + if flag in sys.argv: + idx = sys.argv.index(flag) + if idx + 1 < len(sys.argv): + return sys.argv[idx + 1] + return default + + result = write_prd_review( + sys.argv[2], + findings_json=_prr_flag("findings", "[]"), + blocking_questions_json=_prr_flag("blocking-questions", "[]"), + suggested_revisions_json=_prr_flag("suggested-revisions", "[]"), + route_recommendation=_prr_flag("route-recommendation", ""), + summary=_prr_flag("summary", ""), + prd_source=_prr_flag("prd-source", ""), + branch=_prr_flag("branch") or None, + ) + print(json.dumps(result, indent=2, ensure_ascii=True)) + if result.get("status") == "error": + sys.exit(1) + elif func_name == "write_review_verdict_ledger": # CLI: write_review_verdict_ledger # [--monitor-json ''] diff --git a/src/mapify_cli/templates_src/skills/map-prd-review/SKILL.md.jinja b/src/mapify_cli/templates_src/skills/map-prd-review/SKILL.md.jinja new file mode 100644 index 00000000..2bc94acd --- /dev/null +++ b/src/mapify_cli/templates_src/skills/map-prd-review/SKILL.md.jinja @@ -0,0 +1,152 @@ +--- +name: map-prd-review +description: | + PRD/requirements-quality review before /map-plan. Use when you have a product brief, PRD, or requirements note and need to verify its quality before starting planning. Reviews across 11 dimensions and returns a verdict: ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind. Writes durable artifacts to .map//prd-review.{json,md}. Do NOT use as a substitute for /map-plan, or on well-scoped engineering tasks that have no PRD. +effort: low +disable-model-invocation: true +argument-hint: "[path/to/prd.md or inline requirements text]" +--- +# MAP PRD / Requirements-Quality Review + +Reviews the quality of a supplied PRD, product brief, or requirements note **before** +`/map-plan` transforms it into a spec. A weak PRD causes MAP to invent product decisions, +produce unmeasurable acceptance criteria, and miss security or operational concerns. + +**This skill does NOT write code or start planning.** It produces a quality verdict and +blocking findings that the user resolves before calling `/map-plan`. + +## Input + +Provide one of: +- A path to a Markdown file: `/map-prd-review path/to/feature.md` +- Inline text pasted after the command. + +## Verdicts + +| Verdict | Meaning | Next step | +|---------|---------|-----------| +| `ready_for_plan` | Input is ready for `/map-plan` | Call `/map-plan` | +| `needs_prd_revision` | Fixable missing fields; PRD should be amended | Revise PRD, re-run | +| `needs_user_decision` | Product/design choices must be answered | Answer questions, re-run | +| `route_to_wayfind` | Input too foggy for a PRD review | Call `/map-wayfind` first | + +## Review Dimensions + +The reviewer inspects these dimensions and reports findings: + +1. **measurable_acceptance_criteria** — Are AC items pass/fail testable? +2. **user_job_clarity** — Is the target user and job-to-be-done explicit? +3. **explicit_out_of_scope** — Is what is NOT included stated? +4. **non_functional_requirements** — Are NFRs present when implied by domain/risk? +5. **data_model_lifecycle** — Is data shape, ownership, and lifecycle clear? +6. **ux_states_failure_states** — Are loading, empty, error, and edge UX states covered? +7. **security_trust_boundaries** — Are authentication, authorization, and trust surfaces stated? +8. **dependencies_integrations** — Are external systems and contracts named? +9. **contradictions_assumptions** — Are hidden assumptions or contradictions present? +10. **testability** — Can "done" be verified mechanically? +11. **migration_rollout** — Are migration/rollout/operational concerns addressed when relevant? + +## Effort and Parallelism Policy + +```yaml +thinking_policy: low/direct +parallel_tool_policy: sequential_by_default +``` + +- Run a single focused review; do not spawn sub-reviewers unless the PRD is multi-component. +- Do not write code, modify files outside `.map//`, or start planning. + +## Workflow + +### Step 1: Read the PRD + +Read the supplied file or text. If the argument is a file path that does not exist, report an error and stop. + +### Step 2: Review Against Dimensions + +For each of the 11 dimensions, decide: +- Does the PRD address this dimension adequately? +- If not, classify severity: `critical` (blocks planning), `major` (serious gap), `minor` (nice-to-have), `info` (note). +- Provide a concrete finding description and a suggested revision when applicable. + +### Step 3: Determine Verdict + +Apply this decision logic: + +1. Any `critical` finding → candidate for `needs_prd_revision` or `needs_user_decision`. +2. If the input is so vague/strategic that a PRD review cannot even identify the target user or feature → `route_to_wayfind`. +3. If there are concrete product/design choices that ONLY the human can make → `needs_user_decision` (must produce at least one blocking question). +4. If findings are fixable revisions (missing sections, vague AC) → `needs_prd_revision`. +5. No `critical` or `major` findings → `ready_for_plan`. + +### Step 4: Write Artifacts + +Call the step runner to persist results: + +``` +python3 .map/scripts/map_step_runner.py write_prd_review \ + --findings '' \ + --blocking-questions '' \ + --suggested-revisions '' \ + --summary "" \ + --prd-source "" +``` + +**findings JSON format:** +```json +[ + { + "dimension": "measurable_acceptance_criteria", + "severity": "major", + "description": "AC-2 says 'fast response' but does not specify a latency budget.", + "suggested_revision": "Define latency budget, e.g. p95 < 200ms under 100 RPS." + } +] +``` + +**blocking-questions JSON format (for needs_user_decision):** +```json +[ + { + "question": "Should the feature support unauthenticated users or require login?", + "category": "security_trust_boundaries" + } +] +``` + +**suggested-revisions JSON format (for needs_prd_revision):** +```json +[ + "Add an explicit out-of-scope section listing what this feature does NOT cover.", + "Replace 'should be fast' in AC-3 with a measurable latency target." +] +``` + +### Step 5: Report to User + +Show a summary of the verdict and key findings. If `ready_for_plan`, suggest calling `/map-plan`. + +## Artifacts Written + +- `.map//prd-review.json` — machine-readable verdict and findings +- `.map//prd-review.md` — human-readable review report + +## Examples + +``` +/map-prd-review docs/feature-brief.md +/map-prd-review We want to add a notification system that sends emails when orders ship. +``` + +## Troubleshooting + +- **File not found**: If you pass a path, confirm the file exists relative to the project root before invoking. +- **`route_to_wayfind` returned unexpectedly**: The input is too vague or strategic to review as a PRD. Call `/map-wayfind` first to resolve open decisions, then re-run. +- **Artifact not written**: Check that `.map//` is writable and `map_step_runner.py` is present in `.map/scripts/`. + +## Non-Goals + +- Do not replace `/map-plan`. This reviews the INPUT to planning, not the plan itself. +- Do not block tiny direct edits or well-scoped engineering tasks that have no PRD. +- Do not run multiple agent sub-reviews unless the PRD is unusually large. +- Do not store raw customer data or secrets in `.map/` artifacts. diff --git a/src/mapify_cli/templates_src/skills/skill-rules.json.jinja b/src/mapify_cli/templates_src/skills/skill-rules.json.jinja index 50afd1d6..13d1a1d8 100644 --- a/src/mapify_cli/templates_src/skills/skill-rules.json.jinja +++ b/src/mapify_cli/templates_src/skills/skill-rules.json.jinja @@ -427,6 +427,31 @@ "(find|identify|rank).*(hotspot|shallow|friction)" ] } + }, + "map-prd-review": { + "type": "manual", + "skillClass": "task", + "enforcement": "manual", + "priority": "medium", + "description": "PRD/requirements-quality review before /map-plan: reviews a product brief across 11 dimensions and returns ready_for_plan, needs_prd_revision, needs_user_decision, or route_to_wayfind.", + "promptTriggers": { + "keywords": [ + "map-prd-review", + "prd review", + "requirements review", + "review requirements", + "review prd", + "product brief review", + "requirements quality", + "before map-plan" + ], + "intentPatterns": [ + "map-prd-review", + "(review|audit).*(prd|requirements|product brief|feature brief)", + "(prd|requirements).*(review|quality|ready)", + "before.*(map-plan|planning)" + ] + } } } } diff --git a/tests/test_map_step_runner.py b/tests/test_map_step_runner.py index 90353659..66ae4b6b 100644 --- a/tests/test_map_step_runner.py +++ b/tests/test_map_step_runner.py @@ -15328,3 +15328,328 @@ def test_write_implementer_readiness_review_needs_clarification_requires_blockin assert "blocking_questions" in result["message"].lower() assert "needs_clarification" in result["message"] assert not (branch_workspace / "implementation-readiness.json").exists() + + +# --------------------------------------------------------------------------- +# write_prd_review tests +# --------------------------------------------------------------------------- + +def test_write_prd_review_ready_for_plan_creates_artifacts_and_manifest( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="ready_for_plan", + summary="The PRD is complete and all dimensions pass.", + ) + + assert result["status"] == "success" + assert result["verdict"] == "ready_for_plan" + assert result["proceed"] is True + assert result["findings_count"] == 0 + assert result["blocking_questions_count"] == 0 + assert result["suggested_revisions_count"] == 0 + + json_path = branch_workspace / "prd-review.json" + assert json_path.exists() + payload = json.loads(json_path.read_text(encoding="utf-8")) + assert payload["verdict"] == "ready_for_plan" + assert payload["schema_version"] == "1.0" + assert payload["findings"] == [] + assert payload["blocking_questions"] == [] + assert payload["suggested_revisions"] == [] + + md_path = branch_workspace / "prd-review.md" + assert md_path.exists() + content = md_path.read_text(encoding="utf-8") + assert "READY FOR PLAN" in content + assert "The PRD is complete" in content + + manifest = json.loads((branch_workspace / "artifact_manifest.json").read_text()) + stage = manifest["stages"]["prd_review"] + assert stage["status"] == "ready" + assert stage["metadata"]["verdict"] == "ready_for_plan" + assert stage["metadata"]["findings_count"] == 0 + assert stage["metadata"]["blocking_questions_count"] == 0 + + +def test_write_prd_review_needs_prd_revision_with_findings( + branch_workspace, +): + findings_json = json.dumps([ + { + "dimension": "measurable_acceptance_criteria", + "severity": "major", + "description": "AC-2 says 'fast response' but does not specify a latency budget.", + "suggested_revision": "Define latency budget, e.g. p95 < 200ms under 100 RPS.", + }, + { + "dimension": "explicit_out_of_scope", + "severity": "minor", + "description": "No out-of-scope section is present.", + }, + ]) + suggested_revisions_json = json.dumps([ + "Add an explicit out-of-scope section.", + "Replace 'should be fast' with a measurable latency target.", + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_prd_revision", + findings_json=findings_json, + suggested_revisions_json=suggested_revisions_json, + summary="Two fixable gaps found; PRD should be amended.", + ) + + assert result["status"] == "success" + assert result["verdict"] == "needs_prd_revision" + assert result["proceed"] is False + assert result["findings_count"] == 2 + assert result["suggested_revisions_count"] == 2 + + payload = json.loads( + (branch_workspace / "prd-review.json").read_text(encoding="utf-8") + ) + assert len(payload["findings"]) == 2 + assert payload["findings"][0]["dimension"] == "measurable_acceptance_criteria" + assert payload["findings"][0]["suggested_revision"] == "Define latency budget, e.g. p95 < 200ms under 100 RPS." + assert "suggested_revision" not in payload["findings"][1] + assert payload["suggested_revisions"] == [ + "Add an explicit out-of-scope section.", + "Replace 'should be fast' with a measurable latency target.", + ] + + content = (branch_workspace / "prd-review.md").read_text(encoding="utf-8") + assert "NEEDS PRD REVISION" in content + assert "Findings" in content + assert "measurable_acceptance_criteria" in content + assert "latency budget" in content + assert "Suggested Revisions" in content + + manifest = json.loads((branch_workspace / "artifact_manifest.json").read_text()) + assert manifest["stages"]["prd_review"]["metadata"]["findings_count"] == 2 + assert manifest["stages"]["prd_review"]["metadata"]["suggested_revisions_count"] == 2 + + +def test_write_prd_review_needs_user_decision_with_blocking_questions( + branch_workspace, +): + questions_json = json.dumps([ + { + "question": "Should the feature support unauthenticated users or require login?", + "category": "security_trust_boundaries", + } + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_user_decision", + blocking_questions_json=questions_json, + summary="One product decision must be made by the user before planning.", + ) + + assert result["status"] == "success" + assert result["verdict"] == "needs_user_decision" + assert result["proceed"] is False + assert result["blocking_questions_count"] == 1 + assert "Blocking" in result["message"] or "decision" in result["message"].lower() + + payload = json.loads( + (branch_workspace / "prd-review.json").read_text(encoding="utf-8") + ) + assert len(payload["blocking_questions"]) == 1 + assert payload["blocking_questions"][0]["category"] == "security_trust_boundaries" + + content = (branch_workspace / "prd-review.md").read_text(encoding="utf-8") + assert "NEEDS USER DECISION" in content + assert "Blocking Questions" in content + assert "unauthenticated users" in content + + manifest = json.loads((branch_workspace / "artifact_manifest.json").read_text()) + assert manifest["stages"]["prd_review"]["metadata"]["blocking_questions_count"] == 1 + + +def test_write_prd_review_route_to_wayfind_proceed_is_false( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="route_to_wayfind", + summary="Input is too vague to review as a PRD; wayfinding needed first.", + ) + + assert result["status"] == "success" + assert result["verdict"] == "route_to_wayfind" + assert result["proceed"] is False + assert "wayfind" in result["message"].lower() + + payload = json.loads( + (branch_workspace / "prd-review.json").read_text(encoding="utf-8") + ) + assert payload["verdict"] == "route_to_wayfind" + + content = (branch_workspace / "prd-review.md").read_text(encoding="utf-8") + assert "ROUTE TO WAYFIND" in content + + +def test_write_prd_review_prd_source_stored_in_payload( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="ready_for_plan", + summary="Complete PRD.", + prd_source="docs/feature-brief.md", + ) + + assert result["status"] == "success" + payload = json.loads( + (branch_workspace / "prd-review.json").read_text(encoding="utf-8") + ) + assert payload["prd_source"] == "docs/feature-brief.md" + + content = (branch_workspace / "prd-review.md").read_text(encoding="utf-8") + assert "docs/feature-brief.md" in content + + +def test_write_prd_review_invalid_verdict_returns_error( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="looks_good", + summary="Unknown verdict.", + ) + + assert result["status"] == "error" + assert "looks_good" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_needs_user_decision_requires_blocking_questions( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="needs_user_decision", + blocking_questions_json="[]", + summary="Unclear which path to take.", + ) + + assert result["status"] == "error" + assert "blocking_questions" in result["message"] + assert "needs_user_decision" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_non_array_findings( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="needs_prd_revision", + findings_json='{"dimension": "testability", "severity": "major", "description": "x"}', + summary="Non-array findings.", + ) + + assert result["status"] == "error" + assert "findings must be a JSON array" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_finding_extra_keys( + branch_workspace, +): + findings_json = json.dumps([ + { + "dimension": "testability", + "severity": "major", + "description": "Cannot be verified mechanically.", + "owner": "product", + } + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_prd_revision", + findings_json=findings_json, + summary="Finding has extra key.", + ) + + assert result["status"] == "error" + assert "unsupported fields" in result["message"] + assert "owner" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_invalid_finding_severity( + branch_workspace, +): + findings_json = json.dumps([ + { + "dimension": "user_job_clarity", + "severity": "blocker", + "description": "Target user is not identified.", + } + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_prd_revision", + findings_json=findings_json, + summary="Invalid severity.", + ) + + assert result["status"] == "error" + assert "blocker" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_missing_finding_required_field( + branch_workspace, +): + findings_json = json.dumps([ + { + "dimension": "security_trust_boundaries", + "description": "No authentication model specified.", + # 'severity' is intentionally missing + } + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_prd_revision", + findings_json=findings_json, + summary="Finding missing severity.", + ) + + assert result["status"] == "error" + assert "severity" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_blocking_question_extra_keys( + branch_workspace, +): + questions_json = json.dumps([ + { + "question": "Which pricing model applies?", + "category": "business", + "owner": "product", + } + ]) + + result = map_step_runner.write_prd_review( + verdict="needs_user_decision", + blocking_questions_json=questions_json, + summary="Blocking question has extra key.", + ) + + assert result["status"] == "error" + assert "unsupported fields" in result["message"] + assert "owner" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() + + +def test_write_prd_review_rejects_non_array_blocking_questions( + branch_workspace, +): + result = map_step_runner.write_prd_review( + verdict="needs_user_decision", + blocking_questions_json='{"question": "Who is the user?", "category": "ux"}', + summary="Non-array blocking_questions.", + ) + + assert result["status"] == "error" + assert "blocking_questions must be a JSON array" in result["message"] + assert not (branch_workspace / "prd-review.json").exists() diff --git a/tests/test_skills_consistency.py b/tests/test_skills_consistency.py index 44556f21..2ba2b755 100644 --- a/tests/test_skills_consistency.py +++ b/tests/test_skills_consistency.py @@ -477,13 +477,13 @@ def detect_skill_deps(skill_dir: Path) -> dict[str, set[str]]: def test_skill_discovery_non_empty(skill_names: list[str]) -> None: - """Guard: skill-rules.json must list exactly 20 skills (prevents vacuous pass). + """Guard: skill-rules.json must list exactly 21 skills (prevents vacuous pass). - 20 = the 16 core MAP skills + map-so-search + map-understand + map-wayfind - + map-architecture (#363). + 21 = the 16 core MAP skills + map-so-search + map-understand + map-wayfind + + map-architecture (#363) + map-prd-review (#413). """ - assert len(skill_names) == 20, ( - f"Expected 20 skills in skill-rules.json, found {len(skill_names)}: " + assert len(skill_names) == 21, ( + f"Expected 21 skills in skill-rules.json, found {len(skill_names)}: " f"{sorted(skill_names)}" )