WIP DO NOT MERGE JUST FOR TEST Generate and edit prd - #348
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| def project_prd_path(config: Config) -> pathlib.Path: | ||
| return pathlib.Path(tempfile.gettempdir()) / "softlight-prds" / config.project_id / "prd.md" |
There was a problem hiding this comment.
project_prd_path stores the PRD under tempfile.gettempdir() (typically /tmp on Linux), which is cleared on reboot. The project's spec field now persists this path, not the content. After a system restart, edit_prd will hit prd_path.is_file() == False and raise ValueError("edit-prd expected project spec to be a PRD file path, got ..."), making the edit workflow permanently broken for that project without regenerating the PRD.
| def project_prd_path(config: Config) -> pathlib.Path: | |
| return pathlib.Path(tempfile.gettempdir()) / "softlight-prds" / config.project_id / "prd.md" | |
| def project_prd_path(config: Config) -> pathlib.Path: | |
| return pathlib.Path(config.project_dir) / ".softlight" / "prd.md" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_prd.py
Line: 36-37
Comment:
**PRD written to temp directory — lost on system restart**
`project_prd_path` stores the PRD under `tempfile.gettempdir()` (typically `/tmp` on Linux), which is cleared on reboot. The project's `spec` field now persists this path, not the content. After a system restart, `edit_prd` will hit `prd_path.is_file() == False` and raise `ValueError("edit-prd expected project spec to be a PRD file path, got ...")`, making the edit workflow permanently broken for that project without regenerating the PRD.
```suggestion
def project_prd_path(config: Config) -> pathlib.Path:
return pathlib.Path(config.project_dir) / ".softlight" / "prd.md"
```
How can I resolve this? If you propose a fix, please make it concise.
| resolved = _find_decision(decisions, decision_id) | ||
| other_decisions = [decision for decision in decisions if decision is not resolved] | ||
| transcript = conversation_transcript(project) | ||
| screenshots = conversation_screenshots(project) | ||
|
|
||
| _edit_prd_call( | ||
| config=config, | ||
| prd_file_path=str(prd_path), | ||
| resolved_decision=resolved, | ||
| other_decisions=other_decisions, | ||
| transcript=transcript, | ||
| screenshots=screenshots, | ||
| session_id=f"generate_prd:{run_id}", | ||
| ) |
There was a problem hiding this comment.
edit_prd passes an unresolved decision to Claude without guarding
_find_decision returns any decision by ID regardless of its status. The model prompt asserts "One decision has just been resolved" and instructs Claude to "choose the most consistent interpretation and proceed" when the resolution is ambiguous. If edit_prd is invoked before a decision is actually resolved (no resolved_text, status still "pending"), Claude will synthesise a plausible-sounding but fabricated resolution and rewrite that PRD section, silently corrupting the document with no error raised.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/edit_prd.py
Line: 157-170
Comment:
**`edit_prd` passes an unresolved decision to Claude without guarding**
`_find_decision` returns any decision by ID regardless of its `status`. The model prompt asserts "One decision has just been resolved" and instructs Claude to "choose the most consistent interpretation and proceed" when the resolution is ambiguous. If `edit_prd` is invoked before a decision is actually resolved (no `resolved_text`, `status` still `"pending"`), Claude will synthesise a plausible-sounding but fabricated resolution and rewrite that PRD section, silently corrupting the document with no error raised.
How can I resolve this? If you propose a fix, please make it concise.
| spec_text = project_spec | ||
| path = write_project_prd(config=config, spec=spec_text) | ||
| return path, spec_text |
There was a problem hiding this comment.
project.spec always holds a file path (written by generate_prd), not raw spec text. If that temp file was deleted (reboot, /tmp purge), path.is_file() returns False and the fallback treats the path string itself as spec content — writing e.g. /tmp/softlight-prds/abc/prd.md as the body of a new PRD file. Claude then receives a one-line "PRD" that is just a path string and produces a meaningless prototype. The same bug exists in finalize_initial_prototype.py at lines 99-101. The fallback should raise rather than silently continue with corrupt content.
| spec_text = project_spec | |
| path = write_project_prd(config=config, spec=spec_text) | |
| return path, spec_text | |
| raise ValueError( | |
| f"finalize_prototype PRD file no longer exists at {project_spec!r}; " | |
| "re-run generate-prd to regenerate it", | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/finalize_prototype.py
Line: 99-101
Comment:
After this PR, `project.spec` always holds a file path (written by `generate_prd`), not raw spec text. If that temp file was deleted (reboot, `/tmp` purge), `path.is_file()` returns `False` and the fallback treats the path string itself as spec content — writing e.g. `/tmp/softlight-prds/abc/prd.md` as the body of a new PRD file. Claude then receives a one-line "PRD" that is just a path string and produces a meaningless prototype. The same bug exists in `finalize_initial_prototype.py` at lines 99-101. The fallback should raise rather than silently continue with corrupt content.
```suggestion
raise ValueError(
f"finalize_prototype PRD file no longer exists at {project_spec!r}; "
"re-run generate-prd to regenerate it",
)
```
How can I resolve this? If you propose a fix, please make it concise.| spec_text = project_spec | ||
| path = write_project_prd(config=config, spec=spec_text) | ||
| return path, spec_text | ||
|
|
||
| raise ValueError("finalize_initial_prototype requires a final PRD file path or project spec") |
There was a problem hiding this comment.
finalize_prototype.py: when project.spec holds a temp-file path that no longer exists, spec_text captures the path string itself and write_project_prd writes it as PRD content, silently producing a degenerate one-line spec. Raise an explicit error instead so callers know to re-run generate-prd.
| spec_text = project_spec | |
| path = write_project_prd(config=config, spec=spec_text) | |
| return path, spec_text | |
| raise ValueError("finalize_initial_prototype requires a final PRD file path or project spec") | |
| raise ValueError( | |
| f"finalize_initial_prototype PRD file no longer exists at {project_spec!r}; " | |
| "re-run generate-prd to regenerate it", | |
| ) | |
| raise ValueError("finalize_initial_prototype requires a final PRD file path or project spec") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/finalize_initial_prototype.py
Line: 99-103
Comment:
Same fallback bug as in `finalize_prototype.py`: when `project.spec` holds a temp-file path that no longer exists, `spec_text` captures the path string itself and `write_project_prd` writes it as PRD content, silently producing a degenerate one-line spec. Raise an explicit error instead so callers know to re-run `generate-prd`.
```suggestion
raise ValueError(
f"finalize_initial_prototype PRD file no longer exists at {project_spec!r}; "
"re-run generate-prd to regenerate it",
)
raise ValueError("finalize_initial_prototype requires a final PRD file path or project spec")
```
How can I resolve this? If you propose a fix, please make it concise.| generate_initial_prototype_slot( | ||
| caption_slot_id=caption_slot_id, | ||
| config=config, | ||
| conversations=project.get("conversations", []), | ||
| run_id=run_id, | ||
| session_id=design_session_id(run_id), | ||
| slot_id=slot_id, | ||
| spec=spec, | ||
| spec_path=spec_path, |
There was a problem hiding this comment.
finalize_prototype and finalize_initial_prototype both wrap their core call in try/except BaseException and call _post_slot_error so the UI slot shows an error state when the workflow fails. generate_initial_prototype calls generate_initial_prototype_slot with no such guard. If the prototype build fails (e.g., run_app or call_mcp raises), the create_exploration slot is silently left with no content or error indicator. The @workflow() decorator only handles retries/re-raise — it does not call _post_slot_error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_initial_prototype.py
Line: 380-388
Comment:
**Missing slot error reporting on failure**
`finalize_prototype` and `finalize_initial_prototype` both wrap their core call in `try/except BaseException` and call `_post_slot_error` so the UI slot shows an error state when the workflow fails. `generate_initial_prototype` calls `generate_initial_prototype_slot` with no such guard. If the prototype build fails (e.g., `run_app` or `call_mcp` raises), the `create_exploration` slot is silently left with no content or error indicator. The `@workflow()` decorator only handles retries/re-raise — it does not call `_post_slot_error`.
How can I resolve this? If you propose a fix, please make it concise.2 participants
Greptile Summary
This PR introduces a new
generate_prd/edit_prdtwo-phase workflow where a project-level PRD is generated once and then refined decision-by-decision as the PM resolves open questions. It also addsfinalize_initial_prototypeto apply a final PRD to a prototype and refactors shared utilities into the newcontext.pyandgenerate_sketches.pymodules.generate_prdnow writes the PRD to a temp-directory file and stores its path inproject.spec;edit_prdreads that path and rewrites one decision section in-place after a decision is resolved.generate_initial_prototypeis redesigned to produce a single baseline clone of the existing app (usingos.getcwd()as the source), replacing the previous 3-approach parallel PRD-backed prototype generation.build_decision,next_decision_id,conversation_transcript, etc.) are extracted fromgenerate_decisions.pyinto newgenerate_sketches.pyandcontext.pymodules, removing ~250 lines of duplication.Confidence Score: 3/5
Not safe to merge; multiple unresolved correctness issues in the core PRD persistence and edit-PRD workflow.
The PRD is stored in /tmp and its path is persisted in project.spec. After a system restart or /tmp purge, edit_prd and finalize_initial_prototype will either raise or silently write the raw path string as PRD content, breaking the design workflow permanently for that project without re-running generate_prd. These issues were flagged in the previous review round and remain unresolved in this diff.
generate_prd.py (temp-file PRD persistence), finalize_initial_prototype.py (_read_prd_path silent fallback), and edit_prd.py (unresolved-decision guard).
Important Files Changed
Sequence Diagram
sequenceDiagram participant PM participant generate_prd participant edit_prd participant finalize_initial_prototype participant TmpFS as /tmp (PRD file) participant Claude PM->>generate_prd: trigger generate_prd->>Claude: generate_prd_spec (conversations + decisions) Claude-->>generate_prd: PRD markdown generate_prd->>TmpFS: write_project_prd → prd.md generate_prd-->>PM: "project_updated (spec = /tmp/.../prd.md)" PM->>edit_prd: trigger (decision_id) edit_prd->>TmpFS: read prd.md path from project.spec edit_prd->>Claude: _edit_prd_call (prd_file_path, decisions, hint) Claude->>TmpFS: Read + Edit prd.md directly Claude-->>edit_prd: confirmation edit_prd-->>PM: "project_updated (spec = same path)" PM->>finalize_initial_prototype: trigger (specPath / prototypeDir) finalize_initial_prototype->>TmpFS: _read_prd_path finalize_initial_prototype->>Claude: _edit_final_prototype_app Claude-->>finalize_initial_prototype: summary finalize_initial_prototype-->>PM: slot_updated (iframe prototype)Comments Outside Diff (2)
plugins/softlight/skills/softlight/workflows/generate_decisions.py, line 30-68 (link)prd_anchorwill always be an empty stringbuild_decision()readsraw_decision.get("prd_anchor")and stores it on every decision, but_DECISION_PLAN_SCHEMA(the schema enforced on the Claude call in_generate_decision_plan) does not includeprd_anchorand has"additionalProperties": False. The model can never return this field; it is silently discarded. Every stored decision will carry"prd_anchor": "", making the anchor feature inoperative —edit_prdlocates sections only by the heading shape### Decision <id>: <open_question>and never falls back to the anchor. To activate it,prd_anchormust be added to_DECISION_PLAN_SCHEMA.properties.decisions.itemsand the model prompt must ask for it.Prompt To Fix With AI
plugins/softlight/skills/softlight/workflows/generate_sketches.py, line 852-888 (link)DECISION_ITEM_SCHEMAis defined but never imported or usedDECISION_ITEM_SCHEMAis exported from this module but is not imported bygenerate_decisions.py,edit_prd.py, or any other file touched by this PR. It appears intended as the per-item schema for_DECISION_PLAN_SCHEMA, but that schema still defines its own inline item shape (withoutprd_anchor). Either wire it in or remove it to avoid confusion about the authoritative decision schema.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (5): Last reviewed commit: "initial clone" | Re-trigger Greptile