Skip to content

STU-379: Preserve previous results through refinement and build-error regeneration - #94

Merged
sgardoll merged 2 commits into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/stu-379-preserve-previous-results-through-refinement-and-build-error
Sep 22, 2026
Merged

sgardoll merged 2 commits into
milestone/stu-implement-updated-ui-uxfrom
sgardoll/stu-379-preserve-previous-results-through-refinement-and-build-error

Conversation

@sgardoll

@sgardoll sgardoll commented Sep 22, 2026 •

Copy link
Copy Markdown
Owner

STU-379 — Preserve previous results through refinement and build-error regeneration

Linear: https://linear.app/stuart-gardoll/issue/STU-379/preserve-previous-results-through-refinement-and-build-error

Refinement ("Refine & Regenerate") and pasted-build-error regeneration ("Add FlutterFlow Build Errors & Regenerate") now hold the prior successful result — selected artifact, full bundle, prompt, review, selection — verbatim until a replacement generation AND review both succeed. A generator or reviewer failure restores that prior result and keeps it inspectable and copyable behind a persistent, keyboard-accessible error + retry banner (a toast alone is insufficient). The success path commits the new result atomically and reconciles artifact selection, so no stale tab lingers. Duplicate-click and empty/malformed input are guarded (no double charge, no crash). The pasted-build-error panel is restyled with shared controls, and the actions clearly label their scope (selected artifact vs. whole bundle).

Acceptance criteria — all PASS

  1. Force generator failure and reviewer failure separately; old code remains inspectable/copyable and a retry is available.
    PASS (2 e2e tests, stu379-refinement-regeneration.spec.js):

    • generator failure restores the old code, copyable, with a persistent retry (desktop)
    • reviewer failure restores the old result, not a half-swapped new code (mobile)
      Both assert #results-view stays visible, the old code text is shown (not the new code on reviewer failure), #results-replacement-error is not hidden, #results-replacement-error-retry is visible, and the banner persists past the transient window (unlike a toast). Generator failure also asserts the retry button receives keyboard focus.
  2. Successful refinement swaps in the new result and reconciles selection without stale tabs.
    PASS (2 e2e tests, desktop + mobile): after a successful refinement the new code text replaces the old, #results-summary-detail shows the new review, exactly one artifact tab exists (no stale tabs), selecting it shows the re-generated code, its aria-selected is true, and the banner is hidden. router.generatorCalls() stays 2 (one charge per click).

  3. Cancel/back and malformed/empty error input preserve context; the controls remain keyboard accessible.
    PASS (2 e2e tests, mobile + desktop): empty build-error input is rejected without running the generator (generatorCall count unchanged), no crash, previous result intact; Cancel hides the panel while preserving the typed errors and the previous result. The error textarea gets keyboard focus on open and the retry button is focused on failure.

  4. Mobile and desktop tests cover refinement, pasted build errors, failure recovery and successful replacement.
    PASS: the 8 new tests run across 390×844 and 1440×900 and cover all four journeys:

    • refinement success (mobile + desktop), pasted build-error success (desktop)
    • generator failure + recovery via retry (desktop), reviewer failure (mobile), pasted-error empty/cancel (mobile)

Full CI sequence — ALL GREEN (run at commit 2b96779)

  • npm test → 313 pass / 0 fail
  • npm run build → OK
  • npx playwright test → 121 pass (incl. updated stu377-pipeline-progress.spec.js + new stu379-refinement-regeneration.spec.js)
  • npm run test:accessibility → 7 shells pass
  • npm run test:deploy-guard → 9/9
  • npm run test:proxies → 30/30
  • python3 scripts/scan_bundle_for_leaks.py → no leaks
  • npm run test:key-storage → pass

Falsification evidence (per test, break → FAIL → restore → PASS)

  • Restore-on-failure: temporarily disabled restoreAndShowReplacementFailure (if (true) return;) → the generator-failure, reviewer-failure, and retry tests all FAILED (#results-replacement-error hidden, new code leaked on reviewer failure). Restored → all 3 PASS.
  • Empty-input guard / no charge: changed if (!pastedErrors) to if (false && !pastedErrors) → the empty-input test FAILED (generator called → count 2 ≠ 1). Restored → PASS.
  • Cancel preserves context: changed Cancel to also clearErrorInput() (old behavior) → the cancel-preserves test FAILED (typed text cleared). Restored → PASS.
  • Atomic swap / no stale tabs: no-op'd updateArtifactBundleFromGeneratedCode → the 4 success/swap tests FAILED (new code never committed, generatorCalls wrong). Restored → all PASS.

What could NOT be verified

  • Real paid generation / live FlutterFlow API / real subscription metering were not exercised; per the repo's hermetic testing policy these are mocked via applyDefaultRoutes (deterministic, no network, no wall clock). Commit-on-remote and CI here mean the branch's own tests; the repo has no GitHub Actions run on this branch visible in this session.

Visual evidence (local)

  • /tmp/stu379-desktop-failure-banner.png, /tmp/stu379-mobile-failure-banner.png
  • /tmp/stu379-desktop-success-result.png, /tmp/stu379-mobile-success-result.png
  • /tmp/stu379-desktop-error-input.png, /tmp/stu379-mobile-error-input.png

Note on pre-existing STU-377 tests

Two STU-377 tests asserted the OLD regeneration-failure behaviour (results blanked, #pipeline-failure only). STU-379 explicitly supersedes that ("old code remains inspectable/copyable", "a failure must restore that result plus persistent error/retry UI"), so those two tests were updated to assert the new behaviour while preserving their recoverable-failure + retry-re-enters-Generator→Review intent.

…-error regeneration

Refinement and pasted-build-error regeneration now hold the prior
successful result (selected artifact, full bundle, prompt, review, selection)
until a replacement generation AND review both succeed. A generator or
reviewer failure restores that result behind a persistent error + retry
banner - never a transient toast or a blanked Results surface.

- Snapshot/restore shared pipeline result state around replacement runs
- Commit the new result atomically only after generation + review succeed
- Reconcile artifact selection to the new bundle (no stale tabs)
- Add persistent, keyboard-accessible error + retry banner over the results
- Restyle the pasted-build-errors panel with shared controls; scope labels
- Guard duplicate clicks (no double charge), keep telemetry/usage contracts
@sgardoll
sgardoll marked this pull request as ready for review September 22, 2026 18:33

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Devin Review

Comment thread app.js
restoreGenerationResult(previous);
hidePipelineProgress();
setGenerationStageVisible(true);
showResultsView();

@devin-ai-integration devin-ai-integration Bot Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Failure restoration closes artifact view

When refinement fails from an artifact tab, showResultsView switches the restored result to Summary. The selected artifact panel closes despite restoring its ID.

Learn more

A result has two related selection fields: selectedArtifactId identifies the file, while resultsViewMode decides whether that file or Summary is visible. The snapshot saves only the ID. During failure restoration, showResultsView unconditionally sets resultsViewMode to "summary", so the restored ID no longer owns the visible tab panel.

Example: A user opens artifact B and clicks Refine. If generation fails, artifact B remains in selectedArtifactId, but Results opens on Summary instead of artifact B.

Recommended fix: Include resultsViewMode in snapshotGenerationResult, then repaint the restored state without calling a helper that overwrites that mode. Alternatively, extend showResultsView with an explicit mode-preservation option for restoration only.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread app.js Outdated
Comment on lines +3738 to +3745
if (retryEl) {
retryEl.onclick = () => {
hideReplacementFailure();
if (typeof retry === "function") retry();
};
retryEl.textContent = "Retry";
// Keyboard accessible: a failed replacement lands focus on its retry.
retryEl.focus({ preventScroll: true });

@devin-ai-integration devin-ai-integration Bot Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Non-retryable failures show Retry

Quota or blocked safety failures still expose Retry, despite failure.canRetry being false. Their available upgrade or edit recovery is omitted.

Learn more

The shared error classifier defines which recovery actions apply to each failure. Quota failures set canRetry: false and canUpgrade: true; blocked safety failures set canRetry: false and canEdit: true in classifyPipelineError. The replacement banner ignores those fields and always installs the same retry handler.

Example: A replacement reaches the monthly limit. The retained result displays a Retry button, but repeating the request cannot pass the unchanged limit, and no View plans action is offered.

Recommended fix: Build the banner actions from canRetry, canUpgrade, and canEdit, matching showPipelineFailure. Wire upgrade to openPricingModal and edit to the appropriate prompt-editing flow.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread app.js
restoreGenerationResult(previous);
hidePipelineProgress();
setGenerationStageVisible(true);
showResultsView();

@devin-ai-integration devin-ai-integration Bot Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Restoration clears prior feedback

After feedback was submitted, any replacement failure calls showResultsView and clears that vote. The restored generation can then receive duplicate feedback.

Learn more

The failure path restores the old generation rather than committing a new one. However, showResultsView always calls resetResultsFeedbackState, whose contract is to clear feedback for a new generation. This removes the old result's pressed state, pending lock, and status even though that result remains current.

Example: A user votes thumbs-up, starts refinement, and the review fails. The old code returns with both feedback buttons unpressed, so another thumbs-up sends feedback again for the same generation.

Recommended fix: Repaint a restored result without resetting feedback. Add a preserveFeedback option to showResultsView, or separate result rendering from new-generation initialization and use only the rendering portion here.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@greptile-apps

greptile-apps Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

The PR is not yet safe to merge because the existing blocking stale-failure-banner issue remains unresolved.

Fix All in DevinFindings

  1. P1 Stale failure survives new results ▶
Fix with agent prompt
### Issue 1
app.js:3770-3772
After a replacement fails, this banner is only hidden by Retry or by starting another replacement. If the user edits the prompt and completes a fresh full pipeline, the old failure remains over the new successful result. Its Retry button then runs the old replacement action against the new current result. Clear the replacement failure state when abandoning or resetting a result, or before starting every full pipeline.

Greptile automatically discovered a related ticket stating that persistent retry UI must preserve and act on the prior successful result, which informed this comment.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR preserves the last successful artifact bundle and review while refinement or build-error regeneration runs, restores that result after failure, and atomically presents the replacement after generation and review succeed. Greptile automatically discovered a related ticket that helped explain the purpose of this PR: replacement flows must retain the prior result, provide persistent recovery controls, and avoid stale artifact selection or duplicate generation.

  • Adds snapshot-and-restore handling for replacement pipeline failures.
  • Adds persistent Retry or View plans actions based on the failure type.
  • Reconciles artifact selection after successful refinement.
  • Restyles and clarifies the pasted-build-error and refinement controls.
  • Adds desktop and mobile end-to-end coverage for success, failure, retry, quota, cancellation, and empty input.

Reviews (2) · Last reviewed commit: "fix(STU-379): surface upgrade affordance..."

Comment thread app.js
Comment on lines +3749 to +3751
function hideReplacementFailure() {
const banner = document.getElementById(RESULTS_REPLACEMENT_ERROR_ID);
if (banner) banner.hidden = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale failure survives new results

After a replacement fails, this banner is only hidden by Retry or by starting another replacement. If the user edits the prompt and completes a fresh full pipeline, the old failure remains over the new successful result. Its Retry button then runs the old replacement action against the new current result. Clear the replacement failure state when abandoning or resetting a result, or before starting every full pipeline.

Greptile automatically discovered a related ticket stating that persistent retry UI must preserve and act on the prior successful result, which informed this comment.

Knowledge Base Used: Review and results presentation

Source Used: Linear — Preserve previous results through refinement and build-error regeneration

Prompt To Fix With AI
This is a comment left during a code review.
Path: app.js
Line: 3749-3751

Comment:
**Stale failure survives new results**

After a replacement fails, this banner is only hidden by Retry or by starting another replacement. If the user edits the prompt and completes a fresh full pipeline, the old failure remains over the new successful result. Its Retry button then runs the old replacement action against the new current result. Clear the replacement failure state when abandoning or resetting a result, or before starting every full pipeline.

Greptile automatically discovered a related ticket stating that persistent retry UI must preserve and act on the prior successful result, which informed this comment.

**Knowledge Base Used:** [Review and results presentation](https://app.greptile.com/connect-i-o/-/custom-context/knowledge-base/sgardoll/customcodeconnectforflutterflow/-/docs/review-and-results-presentation.md)

**Source Used:** Linear — [Preserve previous results through refinement and build-error regeneration](https://linear.app/stuart-gardoll/issue/STU-379/preserve-previous-results-through-refinement-and-build-error)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex Fix in Devin

…ent path

A quota/usage-limit failure during refinement or build-error regeneration
rendered only a title, a message, and a retry that re-fired generation into
an already-exhausted allowance — a dead end. Route the replacement-failure
banner through classifyPipelineError's canUpgrade signal so a usage-limit
rejection offers 'View plans' (opening the pricing modal) as the primary
action instead of retry, matching the non-replacement path. The previous
result stays visible and copyable behind the persistent banner, and focus
lands on the action that can resolve the failure. Covers both refinement
and the shared build-error renderer with hermetic browser tests using the
existing quotaExhausted fixture.
@sgardoll
sgardoll merged commit ef4a36a into milestone/stu-implement-updated-ui-ux Sep 22, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant