Progressive wireframe attempt 1 - #356
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "baseUrl": "https://softlight.orianna.ai" | |||
| "baseUrl": "http://localhost:8080" | |||
There was a problem hiding this comment.
This changes the production baseUrl from https://softlight.orianna.ai to http://localhost:8080. Merging this to main means anyone running the plugin from the main branch (CI, staging, or production) will try to reach a local port that doesn't exist in those environments, breaking all API calls. This looks like a dev-only change that was accidentally included in the PR.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/softlight.config.json
Line: 2
Comment:
**Localhost URL committed to shared config**
This changes the production `baseUrl` from `https://softlight.orianna.ai` to `http://localhost:8080`. Merging this to `main` means anyone running the plugin from the main branch (CI, staging, or production) will try to reach a local port that doesn't exist in those environments, breaking all API calls. This looks like a dev-only change that was accidentally included in the PR.
How can I resolve this? If you propose a fix, please make it concise.| for key, value in decision.items() | ||
| if key not in ("open_question", "id") | ||
| if include_decision_sketches or key not in ("sketches", "sketches_ready") | ||
| if include_decision_sketches or key != "sketches" |
There was a problem hiding this comment.
sketches_ready now leaks into context when include_decision_sketches=False
The old filter excluded both sketches and sketches_ready when include_decision_sketches was False. The new filter only excludes sketches, so any existing decisions in the database that still carry sketches_ready (written by the old code) will now appear in the Claude context. While newly created decisions no longer include this field, stale data from projects created before this PR would surface it, potentially confusing the model with an obsolete status flag.
| if include_decision_sketches or key != "sketches" | |
| if include_decision_sketches or key not in ("sketches", "sketches_ready") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/scripts/build_context.py
Line: 127
Comment:
**`sketches_ready` now leaks into context when `include_decision_sketches=False`**
The old filter excluded both `sketches` and `sketches_ready` when `include_decision_sketches` was `False`. The new filter only excludes `sketches`, so any existing decisions in the database that still carry `sketches_ready` (written by the old code) will now appear in the Claude context. While newly created decisions no longer include this field, stale data from projects created before this PR would surface it, potentially confusing the model with an obsolete status flag.
```suggestion
if include_decision_sketches or key not in ("sketches", "sketches_ready")
```
How can I resolve this? If you propose a fix, please make it concise.| screenshot_url = str(((thread.get("screenshot") or {}).get("url")) or "") | ||
| if not screenshot_url: | ||
| continue | ||
| blocks.extend( | ||
| [ | ||
| { | ||
| "type": "text", | ||
| "text": ( | ||
| f"Comment screenshot {index}: slot_id={thread['slot_id']}, " | ||
| f"canvas_x={thread.get('x')}, canvas_y={thread.get('y')}. " | ||
| "The blue dot marks where the comment was placed." | ||
| ), | ||
| }, | ||
| { | ||
| "type": "image", | ||
| "source": {"type": "url", "url": screenshot_url}, | ||
| }, | ||
| ], | ||
| ) | ||
| return blocks | ||
|
|
||
|
|
||
| def _decision_from_raw( | ||
| *, | ||
| raw_decision: dict[str, Any], | ||
| decision_id: str, | ||
| status: str, | ||
| ) -> dict[str, Any]: | ||
| follow_up_questions = [ | ||
| " ".join(str(question).split()) | ||
| for question in (raw_decision.get("follow_up_questions") or []) | ||
| if isinstance(question, str) and question.strip() | ||
| ] | ||
| tradeoffs = [ | ||
| " ".join(str(tradeoff).split()) | ||
| for tradeoff in (raw_decision.get("tradeoffs") or []) | ||
| if isinstance(tradeoff, str) and str(tradeoff).strip() | ||
| ] | ||
| return { | ||
| "id": decision_id, | ||
| "open_question": str(raw_decision["open_question"]).strip(), | ||
| "subtext": str(raw_decision["subtext"]).strip(), | ||
| "sketch_prompt_context": str( | ||
| raw_decision.get("sketch_prompt_context") or "", | ||
| ).strip(), | ||
| "follow_up_questions": follow_up_questions, | ||
| "sketches": [], | ||
| "sketches_ready": False, | ||
| "tradeoffs": tradeoffs, | ||
| "status": status, | ||
| } | ||
| def _prior_wireframes(project: dict[str, Any]) -> list[dict[str, Any]]: | ||
| wireframes: list[dict[str, Any]] = [] | ||
| for revision in project.get("revisions") or []: | ||
| for slot in revision.get("slots") or []: | ||
| element = slot.get("element") or {} | ||
| if element.get("type") != "html": | ||
| continue | ||
| slot_id = str((slot.get("metadata") or {}).get("id") or "") | ||
| if not slot_id: | ||
| continue | ||
| option_id = slot_id.removesuffix("-html") | ||
| wireframes.append( | ||
| { | ||
| "slot_id": slot_id, | ||
| "option_id": option_id, | ||
| "x": slot.get("x"), |
There was a problem hiding this comment.
x/y fields alongside canvas_position
_comment_threads emits top-level x and y keys as well as a nested canvas_position dict containing the exact same values. This duplicate data gets serialised into both the prompt params and the screenshot block text, which adds noise to the prompt sent to Claude. If canvas_position is the authoritative name for coordinates going forward, the bare x/y fields can be removed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_decisions.py
Line: 207-244
Comment:
**Redundant `x`/`y` fields alongside `canvas_position`**
`_comment_threads` emits top-level `x` and `y` keys as well as a nested `canvas_position` dict containing the exact same values. This duplicate data gets serialised into both the prompt params and the screenshot block text, which adds noise to the prompt sent to Claude. If `canvas_position` is the authoritative name for coordinates going forward, the bare `x`/`y` fields can be removed.
How can I resolve this? If you propose a fix, please make it concise.
| exploration = _create_decision_exploration( | ||
| config=config, | ||
| decision=active_decision, | ||
| ) |
There was a problem hiding this comment.
project_updated is posted
_create_decision_exploration and _generate_pass are both called after the project_updated event has already been dispatched. If either throws (e.g., MCP timeout, Claude error, network failure), the decision_updated event never fires, leaving the active decision in the UI with no sketches and no completion signal. The old code wrapped sketch generation in try/except and always posted a decision_updated with sketches_ready: True (even with empty sketches) so the UI could always resolve to a terminal state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_decisions.py
Line: 1142-1145
Comment:
**No error guard after `project_updated` is posted**
`_create_decision_exploration` and `_generate_pass` are both called after the `project_updated` event has already been dispatched. If either throws (e.g., MCP timeout, Claude error, network failure), the `decision_updated` event never fires, leaving the active decision in the UI with no sketches and no completion signal. The old code wrapped sketch generation in try/except and always posted a `decision_updated` with `sketches_ready: True` (even with empty sketches) so the UI could always resolve to a terminal state.
How can I resolve this? If you propose a fix, please make it concise.…ireframes in the product
| requirements_path = path / "requirements-001.md" | ||
| requirements_path.write_text(prd, encoding="utf-8") | ||
|
|
||
| with config.lock: | ||
| config.artifacts.prd_dir = path | ||
| config.artifacts.prd_error = None |
There was a problem hiding this comment.
The old generate_prd_spec rejected a blank Claude response with raise ValueError("generate-prd returned an empty spec"). This version writes whatever prd contains straight to requirements-001.md — including an empty string. Because wait_for_prd_foundation only tests requirements_path.is_file(), a zero-byte (or whitespace-only) file satisfies it, and every downstream caller (edit_prd_result, plan_prototype_approaches) silently proceeds with an empty foundation PRD, producing unusable addenda with no diagnostic.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_prd.py
Line: 170-175
Comment:
**Missing empty-content guard on PRD output**
The old `generate_prd_spec` rejected a blank Claude response with `raise ValueError("generate-prd returned an empty spec")`. This version writes whatever `prd` contains straight to `requirements-001.md` — including an empty string. Because `wait_for_prd_foundation` only tests `requirements_path.is_file()`, a zero-byte (or whitespace-only) file satisfies it, and every downstream caller (`edit_prd_result`, `plan_prototype_approaches`) silently proceeds with an empty foundation PRD, producing unusable addenda with no diagnostic.
How can I resolve this? If you propose a fix, please make it concise.
| for future in concurrent.futures.as_completed(futures): | ||
| options[futures[future]] = future.result() |
There was a problem hiding this comment.
_generate_pass propagates any single option failure without partial results
The concurrent option generation has no per-future error guard. If one of the three generate_option calls raises (e.g., a malformed structured-output response from Claude), future.result() throws and _generate_pass unwinds — the other two completed options are discarded. The old per-sketch try/except at least allowed partial sketches through. Consider catching and logging per-future exceptions so the pass can still return the two healthy options.
| for future in concurrent.futures.as_completed(futures): | |
| options[futures[future]] = future.result() | |
| for future in concurrent.futures.as_completed(futures): | |
| try: | |
| options[futures[future]] = future.result() | |
| except Exception: | |
| import traceback | |
| traceback.print_exc() |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_decisions.py
Line: 1200-1201
Comment:
**`_generate_pass` propagates any single option failure without partial results**
The concurrent option generation has no per-future error guard. If one of the three `generate_option` calls raises (e.g., a malformed structured-output response from Claude), `future.result()` throws and `_generate_pass` unwinds — the other two completed options are discarded. The old per-sketch try/except at least allowed partial sketches through. Consider catching and logging per-future exceptions so the pass can still return the two healthy options.
```suggestion
for future in concurrent.futures.as_completed(futures):
try:
options[futures[future]] = future.result()
except Exception:
import traceback
traceback.print_exc()
```
How can I resolve this? If you propose a fix, please make it concise.
| except Exception as exception: | ||
| with config.lock: | ||
| config.artifacts.prd_error = repr(exception) | ||
| raise | ||
|
|
||
| requirements_path = path / "requirements-001.md" | ||
| requirements_path.write_text(prd, encoding="utf-8") | ||
|
|
||
| with config.lock: | ||
| config.artifacts.prd_dir = path | ||
| config.artifacts.prd_error = None |
There was a problem hiding this comment.
prd_error. If write_text raises (e.g. disk full, permission error) or the subsequent config.lock block throws, prd_error stays None and every caller blocked in wait_for_prd_foundation will silently poll for up to 30 minutes before hitting the timeout rather than failing fast.
| except Exception as exception: | |
| with config.lock: | |
| config.artifacts.prd_error = repr(exception) | |
| raise | |
| requirements_path = path / "requirements-001.md" | |
| requirements_path.write_text(prd, encoding="utf-8") | |
| with config.lock: | |
| config.artifacts.prd_dir = path | |
| config.artifacts.prd_error = None | |
| except Exception as exception: | |
| with config.lock: | |
| config.artifacts.prd_error = repr(exception) | |
| raise | |
| try: | |
| requirements_path = path / "requirements-001.md" | |
| requirements_path.write_text(prd, encoding="utf-8") | |
| with config.lock: | |
| config.artifacts.prd_dir = path | |
| config.artifacts.prd_error = None | |
| except Exception as exception: | |
| with config.lock: | |
| config.artifacts.prd_error = repr(exception) | |
| raise |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_prd.py
Line: 165-175
Comment:
File write and artifact update fall outside the try-except that sets `prd_error`. If `write_text` raises (e.g. disk full, permission error) or the subsequent `config.lock` block throws, `prd_error` stays `None` and every caller blocked in `wait_for_prd_foundation` will silently poll for up to 30 minutes before hitting the timeout rather than failing fast.
```suggestion
except Exception as exception:
with config.lock:
config.artifacts.prd_error = repr(exception)
raise
try:
requirements_path = path / "requirements-001.md"
requirements_path.write_text(prd, encoding="utf-8")
with config.lock:
config.artifacts.prd_dir = path
config.artifacts.prd_error = None
except Exception as exception:
with config.lock:
config.artifacts.prd_error = repr(exception)
raise
```
How can I resolve this? If you propose a fix, please make it concise.
| for future in concurrent.futures.as_completed(futures): | ||
| index = futures[future] | ||
| try: | ||
| sketches[index] = future.result() | ||
| except Exception: | ||
| traceback.print_exc() | ||
|
|
||
| return [sketch for sketch in sketches if sketch is not None] | ||
| sketch_index = futures[future] | ||
| sketch = _sketch_from_option(future.result()) | ||
| sketches[sketch_index] = sketch |
There was a problem hiding this comment.
future.result() leaves fast-preview in broken state
_generate_fast_preview calls future.result() without a try/except. If any one of the three _generate_fast_preview_sketch futures throws (network failure, malformed structured output, MCP timeout), the exception propagates out of the loop, the exploration slots remain empty on the canvas, and neither the fastPreviewReady prompt-progress event nor a decision_updated event is ever sent — leaving the fast-preview decision stuck with no completion signal. The same pattern is already flagged for _generate_pass, but this is the fast lane that runs first for every initial call.
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_decisions.py
Line: 1235-1238
Comment:
**Unguarded `future.result()` leaves fast-preview in broken state**
`_generate_fast_preview` calls `future.result()` without a try/except. If any one of the three `_generate_fast_preview_sketch` futures throws (network failure, malformed structured output, MCP timeout), the exception propagates out of the loop, the exploration slots remain empty on the canvas, and neither the `fastPreviewReady` prompt-progress event nor a `decision_updated` event is ever sent — leaving the fast-preview decision stuck with no completion signal. The same pattern is already flagged for `_generate_pass`, but this is the fast lane that runs first for every initial call.
How can I resolve this? If you propose a fix, please make it concise.| slot_ids = [str(slot_id) for slot_id in exploration["slot_ids"]] | ||
| if len(slot_ids) < 3: | ||
| raise RuntimeError("create_exploration did not return three prototype slots") | ||
| title_slot_id = str(exploration["title_slot_id"]) |
There was a problem hiding this comment.
KeyError on title_slot_id crashes the workflow before any prototype starts
exploration["title_slot_id"] raises KeyError if the create_exploration MCP response doesn't include this new key. Because this line executes before the parallel generate_for_approach tasks are submitted, a crash here leaves three empty exploration slots on the canvas with no error feedback to the user. The adjacent slot_ids validation uses a guarded exploration.get(...) pattern — apply the same to title_slot_id.
| title_slot_id = str(exploration["title_slot_id"]) | |
| title_slot_id = str(exploration.get("title_slot_id") or "") |
Prompt To Fix With AI
This is a comment left during a code review.
Path: plugins/softlight/skills/softlight/workflows/generate_initial_prototypes.py
Line: 140
Comment:
**`KeyError` on `title_slot_id` crashes the workflow before any prototype starts**
`exploration["title_slot_id"]` raises `KeyError` if the `create_exploration` MCP response doesn't include this new key. Because this line executes before the parallel `generate_for_approach` tasks are submitted, a crash here leaves three empty exploration slots on the canvas with no error feedback to the user. The adjacent `slot_ids` validation uses a guarded `exploration.get(...)` pattern — apply the same to `title_slot_id`.
```suggestion
title_slot_id = str(exploration.get("title_slot_id") or "")
```
How can I resolve this? If you propose a fix, please make it concise.1 participant
Greptile Summary
This PR introduces a progressive wireframe system with a dual-lane architecture: a fast-preview lane (Sonnet + no codebase exploration) that fires immediately for initial runs, and a smart lane (Opus planner + Sonnet sketchers with full codebase grounding) that runs in parallel. It also rewires the PRD pipeline so that
generate_prdwrites a foundation document andedit_prdappends per-approach addenda, replacing the old single-shot spec generation.generate_decisions.pyis almost entirely rewritten with new helpers for comment threads, canvas slot management, haiku preview generation, and a threaded flush-on-completion pattern for the initial-mode smart lane.edit_prd.pyis a new file providingplan_prototype_approachesandedit_prd_result, whichgenerate_initial_prototypes.pynow calls instead of generating approaches and PRD specs independently.call_claude.pygains atomic persistence of Claude session IDs to a temp-directory JSON file so sessions survive process restarts within a project.Confidence Score: 3/5
Multiple error-handling gaps in the new concurrent code paths mean the canvas can be left with empty exploration rows and no completion signal when individual sketch or MCP calls fail.
The new
_generate_fast_previewconcurrent loop callsfuture.result()without a try/except, so a single failed sketch aborts the entire fast-preview pass and leaves the prompt progress stuck. Ingenerate_initial_prototypes, a bare dict access onexploration["title_slot_id"]crashes the whole workflow before any parallel prototype task starts. These gaps affect every new code path introduced by this PR.generate_decisions.py(fast-preview concurrent loop),generate_initial_prototypes.py(title_slot_id access), andgenerate_prd.py(file write outside error guard) need the most attention before merging.Important Files Changed
_generate_fast_previewpath that lacks error handling in its concurrent sketch loop — same gap previously flagged for_generate_pass.plan_prototype_approaches+edit_prd_result; introduces an unguardedexploration["title_slot_id"]dict access that crashes the whole workflow before prototypes start if the key is absent.plan_prototype_approachesandedit_prd_result; schema enforcesminLength: 1onrequirements_markdownavoiding the empty-spec risk; logic looks sound.os.replace), but_save_claude_sessionsruns insideconfig.lock, blocking concurrent threads during file I/O.requirements_path.write_textis outside the try-except that setsprd_error, and no empty-content guard on the raw Claude output._as_decision_lines/_as_field_lines/_as_html_lineshelpers;sketches_readyleak wheninclude_decision_sketches=Falsewas previously flagged._transcript_conversationsto strip screenshots from conversation context and passes both spec and conversations to the prototype agent; straightforward and safe.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD GD[generate_decisions called] --> MODE{lane / mode?} MODE -->|fast_preview + initial| FP[_generate_fast_preview] MODE -->|smart + initial| QFP[_queue_fast_preview_prompt] & DP[_generate_decision_plan] MODE -->|smart + revise| RQ{queued canonical?} RQ -->|yes| RV[_reveal_queued_canonical_sketches] RQ -->|no| DP FP --> FPlan[Opus: pick 1 decision + 3 approaches] FPlan --> FCEX[_create_decision_exploration MCP] FCEX --> FSK[3x Sonnet sketches in parallel] FSK -->|any fails| BROKEN([fastPreviewReady never sent]) FSK -->|all succeed| FPRD([prompt_progress fastPreviewReady]) DP --> PLAN[Opus planner: decisions + next_sketch_approaches] PLAN --> EVENTS[post_events: project_updated + prompt_progress] EVENTS --> ACTIVE{active decision?} ACTIVE -->|no| DONE([done]) ACTIVE -->|yes, initial| HK[Haiku previews: 3 options] & SM[Smart sketches: 3 Sonnet options] HK -->|on_preview| FLUSH[flush_available_slot_updates] SM -->|on_option| FLUSH FLUSH --> SLOTS[_create_decision_exploration MCP] SLOTS --> DUPD([decision_updated + slot_updated events]) ACTIVE -->|yes, revise| CEX[_create_decision_exploration MCP] --> HKREV[Haiku previews] & SMREV[_generate_pass Sonnet] SMREV --> DUPD2([decision_updated + slot_updated events])Prompt To Fix All With AI
Reviews (11): Last reviewed commit: "make the whole system feel faster" | Re-trigger Greptile