Skip to content

Working manual generation button. - #304

Merged
ND68 merged 7 commits into
mainfrom
gia/294-manual-summary-generation
Mar 10, 2026
Merged

ND68 merged 7 commits into
mainfrom
gia/294-manual-summary-generation

Conversation

@giannacastr

Copy link
Copy Markdown
Contributor

Description

Resolves ticket number: #294

Explain what your code changes:
I updated the WishlistSummary component to prevent the AI streaming service from triggering automatically when the page loads. I introduced an isRequested state that acts as a gatekeeper, ensuring the API call only fires when the user clicks the toggle. A new button was added next to the filters that matches the primary blue styling used elsewhere in the application to keep the interface consistent (I will look at the styling again to see if I can have it match better).

List the steps you took to test your code:
I added mock data into Wishlist allocation to make sure that when I clicked the button, the wishlist generation would trigger as expected (populating a summary when more than 10 items were present). I did notice that when I pulled from main, the text styling (like bold letters) in the summary did not properly trigger.

Checklist

  • The ticket is mentioned above
  • The changes fulfill the success criteria of the ticket
  • The changes were self-reviewed
  • A review was requested

@giannacastr
giannacastr requested a review from ND68 March 9, 2026 06:20
@giannacastr giannacastr self-assigned this Mar 9, 2026
@netlify

netlify Bot commented Mar 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for hope-for-haiti ready!

Name Link
🔨 Latest commit 4367066
🔍 Latest deploy log https://app.netlify.com/projects/hope-for-haiti/deploys/69b0497dc98d4d000804fa66
😎 Deploy Preview https://deploy-preview-304--hope-for-haiti.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@giannacastr giannacastr linked an issue Mar 9, 2026 that may be closed by this pull request
@greptile-apps

greptile-apps Bot commented Mar 9, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds an on-demand "Generate Summary" button to the WishlistSummary component, preventing the AI streaming service from firing automatically on page load. The change correctly introduces isRequested state as a gate for the stream and restores the "Generate Summary" button after errors so the user can retry — both are solid improvements over the original behaviour.

However, several state-management issues were introduced alongside the UI change:

  • fullText, displayedText, and isLoading are not reset on retry — after an error the user sees garbled output on the second attempt because new stream chunks are appended to the old error string and the typewriter starts mid-way through the combined text.
  • isLoading is initialised to true even though no stream has been requested yet, causing the loading typewriter animation to run silently in the background on every page load before the user has clicked anything.
  • align-top-markdown-content is a non-existent CSS class with no effect anywhere in the codebase.
  • hover:bg-blue-600 (still present in the button) uses Tailwind's default blue palette rather than a project-defined color from the tailwind config (previously flagged).
  • The negative margin trick (-mb-12) and fixed right offset (mr-28) remain fragile layout choices (previously flagged).
  • ReactMarkdown is now wrapping the full displayedText at once rather than line-by-line, which is a cleaner approach, but the inline cursor | is now placed outside the markdown container so it may not render flush with the last line of text.

PR Checklist areas needing improvement

  • No hardcoded colors/sizes — hover:bg-blue-600 uses Tailwind's default palette instead of a config color.
  • Uses flex gaps and paddings instead of margins — -mb-12 and mr-28 are margin-based positioning hacks.
  • Avoids unused imports, variables, or functions — no issues here.
  • Handles error responses from API calls — partially handled; retry is possible, but state is not fully reset between attempts.

Score: 55 / 100

Confidence Score: 2/5

  • Not safe to merge — retrying after a stream error produces garbled output due to unresolved state reset issues.
  • The core gating mechanism works correctly on a first attempt, but the retry path is broken: stale fullText/displayedText cause new stream content to be appended to the old error string, and isLoading not resetting means the loading animation is skipped. These are functional regressions that will be visible to users.
  • src/components/WishlistSummary.tsx requires the most attention — all three logic issues are concentrated there.

Important Files Changed

Filename Overview
src/components/WishlistSummary.tsx Core change adding an isRequested gate for on-demand AI summary generation. Three new bugs introduced: (1) fullText/displayedText/isLoading are not reset on retry, causing garbled content on a second attempt; (2) isLoading initialised to true wastes the loading animation before the user clicks anything; (3) align-top-markdown-content is an undefined CSS class.
src/screens/Wishlist/AdminWishlistScreen.tsx Trivial whitespace-only change: one blank line between <WishlistSummary /> and <AdvancedBaseTable> was removed. No functional impact.

Sequence Diagram

sequenceDiagram
    participant User
    participant WishlistSummary
    participant StreamClient
    participant API

    User->>WishlistSummary: Page loads
    Note over WishlistSummary: isRequested=false, isLoading=true (⚠️ loading animation runs silently)
    WishlistSummary-->>User: Shows "Generate Summary" button

    User->>WishlistSummary: Clicks "Generate Summary"
    WishlistSummary->>WishlistSummary: setIsRequested(true)
    WishlistSummary->>StreamClient: stream("/api/wishlists/summary")
    StreamClient->>API: GET /api/wishlists/summary (SSE)

    alt Happy path
        API-->>StreamClient: onChunk (repeated)
        StreamClient-->>WishlistSummary: setFullText(prev + chunk)
        API-->>StreamClient: onDone
        StreamClient-->>WishlistSummary: setIsLoading(false)
        WishlistSummary-->>User: Typewriter shows summary
    else Error path
        API-->>StreamClient: onError
        StreamClient-->>WishlistSummary: setFullText("Unable to load summary.")
        WishlistSummary->>WishlistSummary: hasStreamed=false, setIsRequested(false)
        WishlistSummary-->>User: Shows "Generate Summary" button again

        User->>WishlistSummary: Clicks "Generate Summary" again
        Note over WishlistSummary: ⚠️ fullText/displayedText NOT reset — old error string still present
        WishlistSummary->>StreamClient: stream("/api/wishlists/summary")
        API-->>StreamClient: onChunk
        StreamClient-->>WishlistSummary: setFullText("Unable to load summary." + chunk)
        Note over WishlistSummary: ⚠️ Typewriter starts at index 23, skipping first 23 chars of summary
        WishlistSummary-->>User: Garbled output shown
    end
Loading

Comments Outside Diff (2)

  1. src/components/WishlistSummary.tsx, line 66-94 (link)

    State not reset on retry — new stream content will be garbled

    When an error occurs, isRequested and hasStreamed.current are correctly reset so the user can click "Generate Summary" again. However, fullText, displayedText, and isLoading are not reset before the second attempt.

    The consequence is:

    1. fullText still holds "Unable to load summary." when the new stream starts. onChunk then does setFullText((prev) => prev + chunk), so the new summary is appended to the old error string.
    2. isLoading is false, so the second useEffect immediately begins typewriting. It starts at index = displayedText.length — i.e. the length of "Unable to load summary." (23 chars). It then calls setDisplayedText(fullText.slice(0, 24)), which slices the combined string of the error + new content, skipping the first 23 characters of the actual summary.
    3. The loading animation ("Loading wishlist summary...") never plays on retry because isLoading never returns to true.

    All three states should be reset when a new stream is started:

    const startStream = async () => {
      hasStreamed.current = true;
      // Reset state for a fresh attempt
      setFullText("");
      setDisplayedText("");
      setIsLoading(true);
      // ... rest of the function
  2. src/components/WishlistSummary.tsx, line 10 (link)

    isLoading initialised to true before the user requests anything

    isLoading starts as true, which means the first useEffect immediately kicks off the "Loading wishlist summary..." typewriter animation in the background — even before the user has ever clicked "Generate Summary". By the time the button is clicked, the full loading string is already in displayedText, so the animation effect is lost and the user sees the completed string appear instantly.

    Since streaming is now opt-in, isLoading should start as false and only be set to true when the stream is actually triggered:

    Then in startStream, set setIsLoading(true) at the top (before the try block) to restore the loading animation for every attempt.

Last reviewed commit: 4367066

Comment thread src/components/WishlistSummary.tsx Outdated
Comment thread src/components/WishlistSummary.tsx Outdated
@ND68

ND68 commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator

Overall functionality looks good. I've updated the styling of the button so no need to worry about that. Please work on fixing the React-markdown issue though.

Comment thread src/components/WishlistSummary.tsx Outdated
Comment thread src/components/WishlistSummary.tsx
giannacastr and others added 2 commits March 10, 2026 12:27
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@ND68

ND68 commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator

@greptileai

Comment thread src/components/WishlistSummary.tsx

@ND68 ND68 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ignore greptile. retries/ generating second summary is not necessary. good work

@ND68
ND68 merged commit e932dd4 into main Mar 10, 2026
5 checks passed
@ND68
ND68 deleted the gia/294-manual-summary-generation branch March 10, 2026 20:28
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.

Manual Generate AI Summary

2 participants