[pro-web] feat: implement logo generation wizard with multi-step creation flow#601
Merged
[pro-web] feat: implement logo generation wizard with multi-step creation flow#601
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Reviewer's GuideImplements a new multi-step Logo Generation wizard integrated with the existing Brand Kit flow, adds a full wordmark/logo-typography system (presets, casing, weights, auto-scaling preview), extends workspace media state/actions for logo data, and introduces precomputed Google Fonts and reference-asset helpers for AI logo generation. Sequence diagram for Brand Kit checkpoint to Logo Generation wizard transitionsequenceDiagram
actor User
participant BrandKitDialog
participant BrandKitWizard
participant CheckpointStep
participant WorkspaceMedia
participant LogoGenerationWizard
User->>BrandKitDialog: open Brand_Kit_dialog
BrandKitDialog->>BrandKitWizard: render view brandKit
User->>BrandKitWizard: complete previous_steps
BrandKitWizard->>CheckpointStep: render CheckpointStep
User->>CheckpointStep: click Generate_Logo
CheckpointStep->>WorkspaceMedia: buildBrandKitData
alt activeOrganization_exists
CheckpointStep->>WorkspaceMedia: saveBrandKit
WorkspaceMedia-->>CheckpointStep: save_success
CheckpointStep->>WorkspaceMedia: clearBrandKitStep
else no_activeOrganization
Note over CheckpointStep,WorkspaceMedia: Skip_saving_brand_kit
end
CheckpointStep->>BrandKitWizard: onGenerateLogo_callback
BrandKitWizard->>BrandKitDialog: onGenerateLogo
BrandKitDialog->>BrandKitDialog: setCurrentView logoGeneration
BrandKitDialog->>LogoGenerationWizard: render LogoGenerationWizard
LogoGenerationWizard->>LogoGenerationWizard: initialize steps Logo_Style,Direction,Refine,Confirm
Sequence diagram for Refine Logo step AI generation with reference assetssequenceDiagram
actor User
participant RefineLogoStep
participant WorkspaceMedia
participant LogoPromptHelper as logo-prompt
participant LogoRefAssetsHelper as logo-reference-assets
participant ImageGen as useImageGeneration
RefineLogoStep->>WorkspaceMedia: get selectedLogoStyle
RefineLogoStep->>WorkspaceMedia: get selectedSymbolStyle
RefineLogoStep->>WorkspaceMedia: get selectedWordmarkStyle
RefineLogoStep->>WorkspaceMedia: get wordmarkName
RefineLogoStep->>WorkspaceMedia: get selectedVibes
RefineLogoStep->>WorkspaceMedia: get selectedPalette
RefineLogoStep->>WorkspaceMedia: get selectedFonts
RefineLogoStep->>LogoRefAssetsHelper: preloadLogoReferenceAssets selectedFonts,selectedPalette,selectedLogoStyle,selectedWordmarkStyle,selectedSymbolStyle
LogoRefAssetsHelper-->>RefineLogoStep: logoReferenceAssets[4]
RefineLogoStep->>WorkspaceMedia: setLogoReferenceAssets logoReferenceAssets
RefineLogoStep->>LogoPromptHelper: buildLogoPrompt selectedLogoStyle,selectedSymbolStyle,selectedWordmarkStyle,wordmarkName,selectedVibes,selectedPalette,selectedFonts
LogoPromptHelper-->>RefineLogoStep: generatedPrompt
RefineLogoStep->>RefineLogoStep: set prompt_if_not_dirty
User->>RefineLogoStep: edit_prompt_optional
User->>RefineLogoStep: click Generate_logo
RefineLogoStep->>LogoRefAssetsHelper: getLogoReferenceChecksums selectedFonts,selectedPalette,selectedLogoStyle,selectedWordmarkStyle,selectedSymbolStyle
LogoRefAssetsHelper-->>RefineLogoStep: referenceImageChecksums[]
RefineLogoStep->>ImageGen: generateImage prompt,gemini-3-pro-image-preview,referenceImageChecksums,aspectRatio_1_1
ImageGen-->>RefineLogoStep: generatedImage_or_error
RefineLogoStep->>WorkspaceMedia: handlePromptSubmit prompt
RefineLogoStep-->>User: show generated_logo_preview_or_error_state
Updated class diagram for workspace media state and brand kit logo typesclassDiagram
class WorkspaceMediaProvider {
+generatedImage
+selectedVibes
+selectedFonts
+selectedPalette
+brandKitCurrentStep
+isSavingBrandKit
+selectedLogoStyle LogoStyle
+selectedSymbolStyle SymbolStyle
+selectedWordmarkStyle WordmarkStyle
+wordmarkName string
+logoReferenceAssets LogoReferenceAsset[]
+wordmarkPreset WordmarkStyle
+wordmarkCasing WordmarkCasing
+wordmarkTracking number
+wordmarkFontWeight WordmarkFontWeight
+setSelectedLogoStyle(style)
+setSelectedSymbolStyle(style)
+setSelectedWordmarkStyle(style)
+setWordmarkName(name)
+setLogoReferenceAssets(assets)
+addLogoReferenceAsset(asset,index)
+removeLogoReferenceAsset(index)
+setWordmarkPreset(preset)
+setWordmarkCasing(casing)
+setWordmarkTracking(tracking)
+setWordmarkFontWeight(weight)
}
class LogoStyle {
<<enumeration>>
+wordmark
+symbol
}
class LogoReferenceAsset {
+type
+previewUrl string
+label string
+metadata Record
}
class SymbolStyle {
<<enumeration>>
+monogram
+geometric-badge
+soft-blob
+sharp-crest
+abstract-symbol
+literal-pictogram
+negative-space
+line-grid
+emblem-seal
+motion-arrow
+mascot-head
+pattern-tile
}
class WordmarkStyle {
<<enumeration>>
+all-caps-tight
+all-caps-wide
+title-case-neutral
+lowercase-soft
+heavy-weight
+light-weight
+condensed
+extended-wide
+small-caps
+custom-letter-tweak
+accent-touch
+color-system-set
}
class WordmarkCasing {
<<enumeration>>
+all-caps
+title-case
+lowercase
+small-caps
}
class WordmarkFontWeight {
<<enumeration>>
+extra_light
+light
+regular
+medium
+semibold
+bold
+black
}
class WordmarkPreset {
+id WordmarkStyle
+title string
+subtitle string
+casingMode
+fixedCasing WordmarkCasing
+trackingMin number
+trackingMax number
+defaultTracking number
+allowedWeights WordmarkFontWeight[]
+defaultWeight WordmarkFontWeight
+notes string
}
class CasingOption {
+id WordmarkCasing
+label string
+icon LucideIcon
}
WorkspaceMediaProvider "1" --> "*" LogoReferenceAsset : manages
WorkspaceMediaProvider --> LogoStyle
WorkspaceMediaProvider --> SymbolStyle
WorkspaceMediaProvider --> WordmarkStyle
WorkspaceMediaProvider --> WordmarkCasing
WorkspaceMediaProvider --> WordmarkFontWeight
WordmarkPreset --> WordmarkStyle
WordmarkPreset --> WordmarkCasing
WordmarkPreset --> WordmarkFontWeight
CasingOption --> WordmarkCasing
Flow diagram for logo reference assets and AI generationflowchart LR
subgraph Brand_Kit_state
A_selectedFonts["Selected fonts in WorkspaceMedia"]
B_selectedPalette["Selected palette in WorkspaceMedia"]
C_logoStyle["selectedLogoStyle"]
D_wordmarkStyle["selectedWordmarkStyle"]
E_symbolStyle["selectedSymbolStyle"]
end
subgraph Refine_Logo_step
F_preload["preloadLogoReferenceAssets"]
G_assets["logoReferenceAssets[4]"]
H_slot["LogoReferenceSlot components"]
I_checksums["getLogoReferenceChecksums"]
end
subgraph Image_generation
J_generate["useImageGeneration.generateImage"]
K_model["Gemini image model"]
end
A_selectedFonts --> F_preload
B_selectedPalette --> F_preload
C_logoStyle --> F_preload
D_wordmarkStyle --> F_preload
E_symbolStyle --> F_preload
F_preload --> G_assets
G_assets --> H_slot
H_slot --> I_checksums
A_selectedFonts --> I_checksums
B_selectedPalette --> I_checksums
C_logoStyle --> I_checksums
D_wordmarkStyle --> I_checksums
E_symbolStyle --> I_checksums
I_checksums --> J_generate
J_generate --> K_model
subgraph Checksum_lookup
L_fonts["BRAND_KIT_CHECKSUMS.fonts"]
M_palettes["BRAND_KIT_CHECKSUMS.palettes"]
N_wordmark["BRAND_KIT_CHECKSUMS.wordmark"]
O_symbol["BRAND_KIT_CHECKSUMS.symbol"]
end
I_checksums --> L_fonts
I_checksums --> M_palettes
I_checksums --> N_wordmark
I_checksums --> O_symbol
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a full brand‑kit and multi‑scope logo generation system: new UI wizards and steps, per‑scope image generation, GCS upload/deduplication for logo assets, checksum-backed reference image mapping, many brand‑kit constants/types, image/base64/ZIP helpers, and refactors to media/workspace hooks and components. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Org API
participant Uploader as uploadLogoAssetToGCS
participant GCS as GoogleCloudStorage
participant GraphQL as GraphQL
participant DB as OrgDataStore
Client->>API: PATCH /api/organizations/:id (brandKit + logoAssets)
API->>API: validate & merge incoming brandKit
API->>Uploader: uploadLogoAssetToGCS(orgId, type, base64)
Uploader->>GCS: compute checksum, check object existence
alt object missing
Uploader->>GCS: upload object
end
GCS-->>Uploader: bucketKey + signedUrl
Uploader-->>API: LogoAsset metadata (url, checksum, bucketKey)
API->>GraphQL: mutate organization { brandKit: updatedBrandKit }
GraphQL->>DB: persist updatedBrandKit
DB-->>GraphQL: mutation result
GraphQL-->>API: updated organization
API-->>Client: 200 OK with transformed org data
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/pro-web/lib/hooks/use-workspace-media.tsx`:
- Around line 759-817: shuffleSelectedPaletteOrder currently builds permutations
from the live selectedPalette colors so once you swap them the next call
permutes the wrong input; fix by storing a canonical color array for each
palette (e.g., a map keyed by selectedPalette.name to the original
[primary,accent,bg,text] created when a palette is first selected) and have
shuffleSelectedPaletteOrder generate permutations from that canonical array
instead of selectedPalette; keep using shuffleState[paletteName] to track the
cursor, update setShuffleState and setSelectedPalette as before (using
nextPerm), and ensure setColorRoleItems still maps positions consistently to
roles (primary->0, accent->1, bg->2, text->3) so selection state remains per
position.
- Around line 297-310: The getGenerationInstance useCallback is being recreated
every render because useImageGeneration() returns fresh objects; fix by
memoizing each generation instance (the results of useImageGeneration()) with
useMemo so wordmarkGeneration, symbolGeneration, and mediaGeneration have stable
references, then keep getGenerationInstance as a useCallback that depends on
those memoized values (or remove useCallback if consumer identity isn't
required); update consumers like logo-generator-wizard.tsx to rely on the
now-stable getGenerationInstance reference.
🧹 Nitpick comments (9)
apps/pro-web/components/routes/pro/chat-panel-pro.tsx (1)
13-29: Several declared props are unused.
onConvertToDocumentis destructured (Line 36) but never referenced. The dialog-state props on Lines 20–28 (convertDialogOpen,setConvertDialogOpen,selectedMessageId,convertedText,setConvertedText,targetProject,setTargetProject,targetDocument,setTargetDocument) are declared in the interface but neither destructured nor passed anywhere. If these are planned forChatPanelProControls, they should be wired through the context or passed as props; otherwise, trim the interface to avoid a misleading public API.apps/pro-web/components/routes/pro/chat-panel-pro-controls.tsx (4)
138-145: AvoidanyforchatOptionsparameter.The
biome-ignoresuppresses the lint warning, butChatRequestOptions(already imported on Line 22) is the expected type here.Proposed fix
const prepareMessageOptions = useCallback( - // biome-ignore lint/suspicious/noExplicitAny: <explanation> - (chatOptions: any) => ({ + (chatOptions: ChatRequestOptions) => ({ ...chatOptions, webSearch: webSearch, }), [webSearch], )
147-178:handleContinueGenerationis recreated every render — consideruseCallback.This async handler is passed to a
Button'sonClick. WithoutuseCallback, a new function reference is created each render, which can cause unnecessary re-renders of child components. The dependencies are stable (getContinuationPrompt,append,chatId,continueGeneration,chatbot,setIsCutOff,setIsContinuing).
309-344: Remove commented-out code blocks.These large commented-out sections (active action indicator, Deep Expertise toggle, web search toggle) add visual noise and reduce readability. If they represent planned features, track them in issues instead.
446-453: Simplify!Boolean(chatbot)to!chatbot.The double negation with
Boolean()is unnecessary for a truthiness check on an optional object, hence the biome lint warning.!chatbotis equivalent and idiomatic.Proposed fix
disabled={ - // biome-ignore lint/complexity/noExtraBooleanCast: <explanation> - !Boolean(chatbot) || + !chatbot || isLoading || isPreProcessing || processingState !== 'idle' }apps/pro-web/lib/hooks/use-workspace-media.tsx (4)
158-175: Extract the logo assets payload type to avoid triple duplication.This exact inline type is repeated in
saveLogoAssetsMutation(lines 634–648) andsaveLogoAssets(lines 713–727). Extract it once and reuse.♻️ Suggested refactor
Define the type once near the other interfaces (or in
brand-kit.types.ts):interface LogoAssetsPayload { symbol?: { base64: string style?: string } wordmark?: { base64: string style?: string name: string fontFamily?: string casing?: string tracking?: number fontWeight?: string } }Then reference
LogoAssetsPayloadin the interface, mutation, and callback.
629-702: Near-identical mutation logic duplicated fromsaveBrandKitMutation.
saveLogoAssetsMutation(lines 629–702) mirrorssaveBrandKitMutation(lines 567–626) almost line-for-line — same endpoint, same IDB sync, same invalidation. Only the request body key differs (brandKitvslogoAssets).Extract the shared PATCH → IDB-sync → invalidate pattern into a helper to reduce ~70 lines of duplication.
♻️ Sketch
async function patchOrganizationAndSyncIDB( organizationId: string, body: Record<string, unknown>, getItem: typeof getItem, updateItem: typeof updateItem, ) { const response = await fetch(`/api/organizations/${organizationId}`, { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }) if (!response.ok) throw new Error(`Failed to save to server`) const result = await response.json() const workspaceData = await getItem(WORKSPACE_STRUCTURE_KEY) if (workspaceData && typeof workspaceData === 'object') { const workspace = workspaceData as unknown as WorkspaceStructure const updatedBrandKit = result.data?.brandKit as BrandKitData | undefined if (updatedBrandKit) { const updatedWorkspace: WorkspaceStructure = { ...workspace, organizationData: workspace.organizationData.map( (org: OrganizationData) => org.id === organizationId ? { ...org, brandKit: updatedBrandKit } : org, ), } await updateItem(WORKSPACE_STRUCTURE_KEY, structureToIDBPayload(updatedWorkspace)) } } return result }Then both mutations become one-liners in
mutationFn.
379-423:handleWordmarkPromptSubmitandhandleSymbolPromptSubmitare near-duplicates — consider a shared helper.The two functions differ only in the scope (
'wordmark'/'symbol') and the generation instance. A single parameterized function would eliminate ~40 lines of duplication:♻️ Sketch
const handleScopedPromptSubmit = async ( scope: 'wordmark' | 'symbol', ) => async (prompt: string) => { const refs = referenceImagesByScope[scope] const checksums = refs.length > 0 ? refs.map(r => r.checksum) : undefined const gen = scope === 'wordmark' ? wordmarkGeneration : symbolGeneration await gen.generateImage(prompt, 'gemini-3-pro-image-preview', { previousImageData: gen.generatedImage || undefined, referenceImageChecksums: checksums, aspectRatio: '1:1', }) }
826-927: Consider splitting this provider — 50+ context values will re-render all consumers on any change.This single context carries image generation state (3 scopes), brand kit state, logo generation state, UI toggles, palette state, and all associated actions. Any state change triggers a re-render of every consumer.
During active image generation (where
isLoading/generatedImageupdate frequently), unrelated consumers (e.g., brand kit step UI) will re-render needlessly. Consider splitting into focused contexts (e.g.,LogoGenerationContext,BrandKitContext) or memoizing sub-objects withuseMemo.
Comment on lines
+297
to
+310
| // Three separate image generation instances | ||
| const mediaGeneration = useImageGeneration() | ||
| const wordmarkGeneration = useImageGeneration() | ||
| const symbolGeneration = useImageGeneration() | ||
|
|
||
| // Helper to get the correct instance based on scope | ||
| const getGenerationInstance = useCallback( | ||
| (scope: GenerationScope) => { | ||
| if (scope === 'wordmark') return wordmarkGeneration | ||
| if (scope === 'symbol') return symbolGeneration | ||
| return mediaGeneration | ||
| }, | ||
| [wordmarkGeneration, symbolGeneration, mediaGeneration], | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the useImageGeneration hook implementation
fd "use-image-generation" --type f -e ts -e tsxRepository: bitcashorg/masterbots
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Read the useImageGeneration hook implementation
cat -n apps/pro-web/lib/hooks/use-image-generation.tsxRepository: bitcashorg/masterbots
Length of output: 9453
🏁 Script executed:
#!/bin/bash
# Check how getGenerationInstance is used in the file
cat -n apps/pro-web/lib/hooks/use-workspace-media.tsx | grep -A 5 -B 5 "getGenerationInstance"Repository: bitcashorg/masterbots
Length of output: 1268
🏁 Script executed:
#!/bin/bash
# Check the full context of how getGenerationInstance is used and what context it's part of
sed -n '290,330p' apps/pro-web/lib/hooks/use-workspace-media.tsxRepository: bitcashorg/masterbots
Length of output: 1649
🏁 Script executed:
#!/bin/bash
# Check where getGenerationInstance is consumed - look for useEffect/useCallback dependencies
rg "getGenerationInstance" apps/pro-web/lib/hooks/use-workspace-media.tsx -A 3 -B 1Repository: bitcashorg/masterbots
Length of output: 614
🏁 Script executed:
#!/bin/bash
# Check the full return value of this hook to see the context object structure
sed -n '860,950p' apps/pro-web/lib/hooks/use-workspace-media.tsxRepository: bitcashorg/masterbots
Length of output: 1914
🏁 Script executed:
#!/bin/bash
# Look for where getGenerationInstance is actually consumed/used
# Check if it's used in useEffect dependencies or passed to children
rg "getGenerationInstance" apps/pro-web --type tsx --type ts -B 2 -A 2Repository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Look for where getGenerationInstance is actually consumed/used
rg "getGenerationInstance" apps/pro-web -A 2 -B 2Repository: bitcashorg/masterbots
Length of output: 8949
useCallback for getGenerationInstance is ineffective due to unstable dependencies and causes downstream effects to re-run unnecessarily.
useImageGeneration() returns a new object reference on every render and is not memoized. This means the dependency array [wordmarkGeneration, symbolGeneration, mediaGeneration] always contains new references, making the useCallback recreate the function on every render. Additionally, consumers like logo-generator-wizard.tsx include getGenerationInstance in useEffect dependencies, causing those effects to re-run every render instead of only when actual logic changes.
Memoize the return value of useImageGeneration() using useMemo to provide stable references, or alternatively remove the useCallback if function identity is not critical for consumers.
🤖 Prompt for AI Agents
In `@apps/pro-web/lib/hooks/use-workspace-media.tsx` around lines 297 - 310, The
getGenerationInstance useCallback is being recreated every render because
useImageGeneration() returns fresh objects; fix by memoizing each generation
instance (the results of useImageGeneration()) with useMemo so
wordmarkGeneration, symbolGeneration, and mediaGeneration have stable
references, then keep getGenerationInstance as a useCallback that depends on
those memoized values (or remove useCallback if consumer identity isn't
required); update consumers like logo-generator-wizard.tsx to rely on the
now-stable getGenerationInstance reference.
Comment on lines
+759
to
+817
| // Shuffle selected palette order deterministically | ||
| const shuffleSelectedPaletteOrder = useCallback(() => { | ||
| if (!selectedPalette) return | ||
|
|
||
| const paletteName = selectedPalette.name | ||
| const baseColors = [ | ||
| selectedPalette.primary, | ||
| selectedPalette.accent, | ||
| selectedPalette.bg, | ||
| selectedPalette.text, | ||
| ] | ||
|
|
||
| // Generate all valid permutations | ||
| const allPermutations = generatePermutations(baseColors) | ||
|
|
||
| // Filter to valid combinations only | ||
| const validPermutations = allPermutations.filter((perm) => { | ||
| const [, , bg, text] = perm | ||
| // bg and text must have good contrast (WCAG AA) | ||
| return isValidTextCombination(bg, text) | ||
| }) | ||
|
|
||
| if (validPermutations.length === 0) { | ||
| console.warn('[shuffleSelectedPaletteOrder] No valid permutations found') | ||
| return | ||
| } | ||
|
|
||
| // Get current cursor for this palette (default to 0) | ||
| const currentCursor = shuffleState[paletteName] ?? 0 | ||
|
|
||
| // Advance to next valid permutation (cycle) | ||
| const nextCursor = (currentCursor + 1) % validPermutations.length | ||
| const nextPerm = validPermutations[nextCursor] | ||
|
|
||
| // Update shuffle state | ||
| setShuffleState((prev) => ({ ...prev, [paletteName]: nextCursor })) | ||
|
|
||
| // Update selectedPalette with new color assignments | ||
| setSelectedPalette({ | ||
| name: selectedPalette.name, | ||
| category: selectedPalette.category, | ||
| primary: nextPerm[0], | ||
| accent: nextPerm[1], | ||
| bg: nextPerm[2], | ||
| text: nextPerm[3], | ||
| }) | ||
|
|
||
| // Also update colorRoleItems to match the new palette order | ||
| // Keep selection state per position | ||
| setColorRoleItems((prev) => { | ||
| const selections = prev.map((item) => item.selected) | ||
| return [ | ||
| { role: 'primary', color: nextPerm[0], selected: selections[0] }, | ||
| { role: 'secondary', color: nextPerm[2], selected: selections[1] }, // bg maps to secondary | ||
| { role: 'accent', color: nextPerm[1], selected: selections[2] }, | ||
| { role: 'muted', color: nextPerm[3], selected: selections[3] }, // text maps to muted | ||
| ] | ||
| }) | ||
| }, [selectedPalette, shuffleState]) |
Contributor
There was a problem hiding this comment.
Bug: Cursor-based shuffle breaks after the first invocation because input colors change.
shuffleSelectedPaletteOrder generates permutations from selectedPalette's current colors (line 764–769), then updates selectedPalette with the chosen permutation (line 797–804). On the next call, baseColors reflects the already-shuffled order, producing a completely different permutation set — the stored cursor (shuffleState[paletteName]) now indexes into the wrong list.
Effect: After the first shuffle, subsequent shuffles will not cycle through all valid permutations deterministically; they may repeat, skip, or land on unexpected orderings.
Fix: Derive permutations from the palette's original/canonical color ordering (e.g., store the initial [primary, accent, bg, text] once per palette) rather than the live selectedPalette state.
🤖 Prompt for AI Agents
In `@apps/pro-web/lib/hooks/use-workspace-media.tsx` around lines 759 - 817,
shuffleSelectedPaletteOrder currently builds permutations from the live
selectedPalette colors so once you swap them the next call permutes the wrong
input; fix by storing a canonical color array for each palette (e.g., a map
keyed by selectedPalette.name to the original [primary,accent,bg,text] created
when a palette is first selected) and have shuffleSelectedPaletteOrder generate
permutations from that canonical array instead of selectedPalette; keep using
shuffleState[paletteName] to track the cursor, update setShuffleState and
setSelectedPalette as before (using nextPerm), and ensure setColorRoleItems
still maps positions consistently to roles (primary->0, accent->1, bg->2,
text->3) so selection state remains per position.
Member
There was a problem hiding this comment.
Good to dive deep and test edge cases in case they shuffle a lot or one or more colours are disabled
Contributor
There was a problem hiding this comment.
@AndlerRL, great point about the edge cases! Let me break down the current behavior and potential issues:
Current Issues & Edge Cases
1. Main Bug (Shuffling Multiple Times)
As CodeRabbit identified, after the first shuffle, the baseColors array is built from already-shuffled colors. This means:
- 1st shuffle: permutes
[primary, accent, bg, text]✅ - 2nd shuffle: permutes whatever the 1st shuffle produced ❌
- 3rd+ shuffles: increasingly unpredictable ❌
The cursor becomes meaningless because it's indexing into a different permutation set each time.
2. Disabled Colors Edge Case
Currently, shuffleSelectedPaletteOrder generates permutations from all 4 colors regardless of which ones are selected/disabled in colorRoleItems. The selection state is preserved per position (lines 811-817), but disabled colors still participate in the shuffle.
Question: When one or more colors are disabled, should the shuffle:
- Option A: Only permute the selected/enabled colors, leaving disabled ones in their positions?
- Option B: Permute all 4 colors but only show selected ones in the UI (current behavior)?
3. Empty Permutations
If all permutations fail the WCAG contrast check, the function warns and exits (line 781-782), but leaves the palette in whatever state it was in.
Suggested Fix
To address the main bug and these edge cases:
- Store canonical colors when a palette is first selected
- Generate permutations from canonical colors (not current shuffled state)
- Respect disabled colors (if desired) by filtering permutations or only shuffling selected colors
- Handle edge cases: empty valid permutations, rapid multiple shuffles
Would you like me to provide implementation guidance for these fixes? Also, please clarify the desired behavior for disabled colors during shuffle (Option A or B above).
🧠 Learnings used
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2026-01-22T23:10:37.056Z
Learning: AndlerRL emphasizes Modular Development: avoid duplication by centralizing shared utilities/types, prefer small composable units, and keep flows simple and explicit (no cumbersome validations or over-engineering).
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 542
File: apps/web/components/layout/profile/layout.tsx:67-68
Timestamp: 2025-08-29T20:34:24.433Z
Learning: AndlerRL prefers skeleton loading states over conditional rendering for better UX when components are waiting for data to load. Components should show loading indicators rather than blank spaces.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:221-221
Timestamp: 2026-01-22T23:03:44.735Z
Learning: In TSX files (e.g., apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx), adopt a convention where variables that reference DOM elements are prefixed with '$' (e.g., $questionElement, $el) to quickly identify DOM refs. Apply this consistently across the codebase to improve readability and searchability; ensure the prefix is only used for actual DOM element references and not for non-DOM variables.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:40-40
Timestamp: 2026-01-22T23:03:49.088Z
Learning: In TypeScript React files, prefer using optional chaining with a nullish coalescing fallback (e.g., object?.param ?? fallback) over explicit type-guard helpers like isObject or asType when accessing nested properties. This keeps code simpler and more idiomatic. Ensure you still handle cases where intermediate properties may be undefined before applying the ?? fallback.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:221-221
Timestamp: 2026-01-22T23:03:43.504Z
Learning: Enforce a DOM element naming convention in the pro-web app: in TSX files (e.g., apps/pro-web/**/*.tsx), prefix variables that hold DOM element references with a leading '$'. For example: const $questionElement = document.getElementById('question'); or const $list = document.querySelector('.list'); This helps quickly identify DOM refs and distinguish them from other variables. Apply this to variables obtained via document.getElementById, querySelector, etc., and ensure non-DOM variables are not prefixed.
derianrddev
force-pushed
the
feat/logo-generator
branch
from
0534dc6 to
80a113e
Compare
derianrddev
commented
Feb 13, 2026
| "format:write": "prettier --write \"{app,lib,components}/**/*.{ts,tsx,mdx}\" --cache", | ||
| "format:check": "prettier --check \"{app,lib,components}**/*.{ts,tsx,mdx}\" --cache" | ||
| "format:check": "prettier --check \"{app,lib,components}**/*.{ts,tsx,mdx}\" --cache", | ||
| "upload-brand-kit-assets": "bun run scripts/upload-brand-kit-assets.ts" |
Contributor
Author
There was a problem hiding this comment.
Note: With the current approach, if the GCS environment variables change or the bucket is cleared/removed, this script needs to be run again to re-upload the base Brand Kit assets (logo styles, fonts, and palettes).
TODO: Investigate whether there is a better “seed” or automatic preload strategy for assets in GCS, or move this step to CI/CD in the future to avoid manual dependency.
CC: @AndlerRL
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/pro-web/components/routes/chat/prompt-form/index.tsx (1)
509-616:⚠️ Potential issue | 🟠 MajorAsync submission handlers lack error handling — unhandled rejections possible.
handleWordmarkPromptSubmit,handleSymbolPromptSubmit,handleMediaPromptSubmit, andonSubmitare allawaited but none are wrapped intry/catch. If any of these reject, the promise rejection goes unhandled. Combined with the prematuresetInput(''), the user loses their input with no feedback.Wrap the async calls (or the entire function body after validation) in a
try/catchthat shows an error viacustomSonnerand restores the input on failure.As per coding guidelines: "Handle GraphQL operation failures at network, parsing, and business logic levels with meaningful fallbacks for degraded functionality" and "Test assumptions about API calls and DOM operations - don't assume success, always verify results and provide meaningful fallbacks."
🤖 Fix all issues with AI agents
In `@apps/pro-web/components/routes/chat/prompt-form/index.tsx`:
- Around line 334-340: getGenerationInstance is being recreated every render
because mediaGeneration, wordmarkGeneration, and symbolGeneration are new
objects each time (they come from direct useImageGeneration() calls), so memoize
those three instances with useMemo and then keep getGenerationInstance wrapped
with useCallback depending on the memoized values; specifically, replace direct
useImageGeneration() invocations with memoized variables (e.g., const
mediaGeneration = useMemo(() => useImageGeneration(...), [...deps]) or memoize
the returned objects), update getGenerationInstance's dependency array to
reference those memoized symbols (mediaGeneration, wordmarkGeneration,
symbolGeneration), and this will stabilize getGenerationInstance so the effect
that depends on it (the useEffect at the block ending with setInput) no longer
re-runs every render.
- Around line 178-179: The current logic allows a fragile state where
isBrandKitOpen can be true while activeGenerationScope remains 'media', making
the media submission path unreachable; update state coordination by either (A)
when toggling brand kit open (where isBrandKitOpen is set), also set
activeGenerationScope to a valid input scope like 'wordmark' or 'symbol' (update
the brand kit open/close handler that mutates isBrandKitOpen and
activeGenerationScope), or (B) add an explicit guard/reset where
isBrandKitInputDisabled is computed or in the submit handlers (e.g., the media
submission handler) to detect the invalid combination (isBrandKitOpen &&
activeGenerationScope === 'media') and reset activeGenerationScope to a safe
default before proceeding; reference isBrandKitOpen, activeGenerationScope,
isBrandKitInputDisabled, and the media submission handler to locate where to
apply the change.
🧹 Nitpick comments (1)
apps/pro-web/components/routes/chat/prompt-form/index.tsx (1)
322-333: DOM element variable should use$prefix per codebase convention.Line 325:
elshould be$elto follow the pro-web convention of prefixing DOM element references with$.Proposed fix
setTimeout(() => { if (!inputRef.current) return - const el = inputRef.current - el.focus() + const $el = inputRef.current + $el.focus() // Move caret to end - el.setSelectionRange(draft.length, draft.length) + $el.setSelectionRange(draft.length, draft.length) // Scroll to bottom - el.scrollTop = el.scrollHeight + $el.scrollTop = $el.scrollHeight }, 0)Based on learnings: "In TSX files, adopt a convention where variables that reference DOM elements are prefixed with '$' (e.g., $questionElement, $el) to quickly identify DOM refs."
Comment on lines
+178
to
+179
| const isBrandKitInputDisabled = | ||
| isBrandKitOpen && !isWordmarkScope && !isSymbolScope |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file in question to understand the context better
fd 'prompt-form' --type f -e tsx | head -5Repository: bitcashorg/masterbots
Length of output: 47
🏁 Script executed:
# Search for where activeGenerationScope is defined/assigned
rg -n 'activeGenerationScope\s*=' --type=ts --type=tsx -B 2 -A 2Repository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Search for isBrandKitOpen and its assignments
rg -n 'isBrandKitOpen' --type=ts --type=tsx -B 2 -A 2 | head -100Repository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Look for media scope handling
rg -n "media" --type=ts --type=tsx -i -B 2 -A 2 | grep -A 5 -B 5 "scope\|media" | head -80Repository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Find the file directly
find . -path "*prompt-form/index.tsx" -type fRepository: bitcashorg/masterbots
Length of output: 180
🏁 Script executed:
# Search for activeGenerationScope without file type specification
rg -n 'activeGenerationScope' -B 2 -A 2Repository: bitcashorg/masterbots
Length of output: 3486
🏁 Script executed:
# Search for isBrandKitOpen assignments
rg -n 'isBrandKitOpen.*=' -B 1 -A 1 | head -100Repository: bitcashorg/masterbots
Length of output: 1339
🏁 Script executed:
# Look at the prompt-form component structure
find . -path "*prompt-form*" -type f | grep -E "\.(ts|tsx|js|jsx)$"Repository: bitcashorg/masterbots
Length of output: 536
🏁 Script executed:
# Search for handleMediaPromptSubmit to understand media scope handling
rg -n 'handleMediaPromptSubmit' -B 3 -A 5Repository: bitcashorg/masterbots
Length of output: 3892
🏁 Script executed:
# Search for setActiveGenerationScope calls to understand when scope changes
rg -n 'setActiveGenerationScope' -B 3 -A 3Repository: bitcashorg/masterbots
Length of output: 4088
🏁 Script executed:
# Look at the BrandKitFlow and how it manages state
rg -n 'BrandKitFlow' -B 2 -A 10Repository: bitcashorg/masterbots
Length of output: 4973
🏁 Script executed:
# Check lines around 178-179 in the actual prompt-form file to see full context
sed -n '165,200p' ./apps/pro-web/components/routes/chat/prompt-form/index.tsxRepository: bitcashorg/masterbots
Length of output: 1538
🏁 Script executed:
# Check lines 595-650 to see the submitPrompt logic and media scope handling
sed -n '590,650p' ./apps/pro-web/components/routes/chat/prompt-form/index.tsxRepository: bitcashorg/masterbots
Length of output: 1948
🏁 Script executed:
# Check if activeGenerationScope changes based on isBrandKitOpen
rg -n 'isBrandKitOpen' -B 5 -A 5Repository: bitcashorg/masterbots
Length of output: 10744
🏁 Script executed:
# Search for where brand kit state might affect activeGenerationScope
rg -n 'setIsBrandKitOpen' -B 5 -A 5Repository: bitcashorg/masterbots
Length of output: 5817
🏁 Script executed:
# Check if there's any effect that manages scope when brand kit changes
sed -n '1,100p' ./apps/pro-web/lib/hooks/use-workspace-media.tsx | head -50Repository: bitcashorg/masterbots
Length of output: 1479
🏁 Script executed:
# Look for any effects that coordinate isBrandKitOpen with activeGenerationScope
rg -n 'useEffect' ./apps/pro-web/lib/hooks/use-workspace-media.tsx -A 10 | head -80Repository: bitcashorg/masterbots
Length of output: 47
🏁 Script executed:
# Check the textarea disabled prop to confirm the condition
sed -n '795,820p' ./apps/pro-web/components/routes/chat/prompt-form/index.tsxRepository: bitcashorg/masterbots
Length of output: 951
🏁 Script executed:
# Check if there's any logic that prevents media scope when brand kit opens, or verifies the UI flow
sed -n '35,120p' ./apps/pro-web/components/routes/pro/chat-panel-pro.tsxRepository: bitcashorg/masterbots
Length of output: 2282
🏁 Script executed:
# Look at BrandKitFlow to understand how it manages scope
sed -n '20,80p' ./apps/pro-web/components/routes/workspace/media-tab/brand-kit/brand-kit-flow.tsxRepository: bitcashorg/masterbots
Length of output: 1687
🏁 Script executed:
# Check if there's documentation or comments explaining the intended relationship
rg -n 'Brand.*Kit.*scope|media.*Brand.*Kit' -B 2 -A 2Repository: bitcashorg/masterbots
Length of output: 47
Clarify state coordination between isBrandKitOpen and activeGenerationScope to prevent unreachable media submission path.
The input disables when isBrandKitOpen && !isWordmarkScope && !isSymbolScope (i.e., when scope is 'media'). While the prompt form is currently hidden entirely when brand kit opens (chat-panel-pro.tsx line 97), there's no explicit guarantee that activeGenerationScope changes from its default 'media' value. This creates a fragile dependency where the media submission handler (lines 595–597) could become unreachable if the UI architecture changes. Consider either: (1) explicitly setting activeGenerationScope to 'wordmark' or 'symbol' when brand kit opens, or (2) adding an explicit guard/reset mechanism to prevent this state combination.
🤖 Prompt for AI Agents
In `@apps/pro-web/components/routes/chat/prompt-form/index.tsx` around lines 178 -
179, The current logic allows a fragile state where isBrandKitOpen can be true
while activeGenerationScope remains 'media', making the media submission path
unreachable; update state coordination by either (A) when toggling brand kit
open (where isBrandKitOpen is set), also set activeGenerationScope to a valid
input scope like 'wordmark' or 'symbol' (update the brand kit open/close handler
that mutates isBrandKitOpen and activeGenerationScope), or (B) add an explicit
guard/reset where isBrandKitInputDisabled is computed or in the submit handlers
(e.g., the media submission handler) to detect the invalid combination
(isBrandKitOpen && activeGenerationScope === 'media') and reset
activeGenerationScope to a safe default before proceeding; reference
isBrandKitOpen, activeGenerationScope, isBrandKitInputDisabled, and the media
submission handler to locate where to apply the change.
Comment on lines
+334
to
+340
| }, [ | ||
| isWorkspaceActive, | ||
| activeGenerationScope, | ||
| selectedSymbolStyle, | ||
| getGenerationInstance, | ||
| setInput, | ||
| ]) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the useWorkspaceMedia hook definition
fd 'use-workspace-media' --type f -e ts -e tsxRepository: bitcashorg/masterbots
Length of output: 111
🏁 Script executed:
# Also search for useWorkspaceMedia in the codebase
rg 'useWorkspaceMedia' --type ts --type tsx -lRepository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Search for getGenerationInstance declarations and usage
rg 'getGenerationInstance' --type ts --type tsx -B2 -A2 | head -150Repository: bitcashorg/masterbots
Length of output: 92
🏁 Script executed:
# Check the specific file and lines mentioned in the review
cat -n apps/pro-web/components/routes/chat/prompt-form/index.tsx | sed -n '170,180p;330,345p'Repository: bitcashorg/masterbots
Length of output: 1029
🏁 Script executed:
# Read the useWorkspaceMedia hook to check if getGenerationInstance is wrapped in useCallback
cat -n apps/pro-web/lib/hooks/use-workspace-media.tsxRepository: bitcashorg/masterbots
Length of output: 34237
🏁 Script executed:
# Also search for where getGenerationInstance is derived from or defined
rg 'getGenerationInstance\s*=' --type ts --type js -B5 -A5Repository: bitcashorg/masterbots
Length of output: 991
🏁 Script executed:
# Check the exact hook definition and its return
rg 'export.*useWorkspaceMedia' --type ts --type js -A30Repository: bitcashorg/masterbots
Length of output: 719
🏁 Script executed:
# Find useImageGeneration hook
fd 'use-image-generation' --type f -e ts -e tsxRepository: bitcashorg/masterbots
Length of output: 156
🏁 Script executed:
# Search for useImageGeneration definition
rg 'export.*useImageGeneration|function useImageGeneration' --type ts --type js -A 20Repository: bitcashorg/masterbots
Length of output: 3504
🏁 Script executed:
# Read the full useImageGeneration hook from pro-web
cat -n apps/pro-web/lib/hooks/use-image-generation.tsx | head -200Repository: bitcashorg/masterbots
Length of output: 6865
🏁 Script executed:
# Look for the return statement of useImageGeneration
rg 'return \{' apps/pro-web/lib/hooks/use-image-generation.tsx -A 30Repository: bitcashorg/masterbots
Length of output: 286
🏁 Script executed:
# Check if the generation instances are wrapped in useMemo in useWorkspaceMedia
rg -n 'mediaGeneration|wordmarkGeneration|symbolGeneration' apps/pro-web/lib/hooks/use-workspace-media.tsx | head -20Repository: bitcashorg/masterbots
Length of output: 1083
Memoize the generation instances to prevent getGenerationInstance from being recreated on every render.
getGenerationInstance is wrapped in useCallback, but its dependency array references mediaGeneration, wordmarkGeneration, and symbolGeneration—three new object instances created on every render via direct useImageGeneration() calls. This causes getGenerationInstance itself to be recreated every render despite useCallback, making the effect at line 334 re-run wastefully. Wrap the three generation instances in useMemo to stabilize the dependency chain.
🤖 Prompt for AI Agents
In `@apps/pro-web/components/routes/chat/prompt-form/index.tsx` around lines 334 -
340, getGenerationInstance is being recreated every render because
mediaGeneration, wordmarkGeneration, and symbolGeneration are new objects each
time (they come from direct useImageGeneration() calls), so memoize those three
instances with useMemo and then keep getGenerationInstance wrapped with
useCallback depending on the memoized values; specifically, replace direct
useImageGeneration() invocations with memoized variables (e.g., const
mediaGeneration = useMemo(() => useImageGeneration(...), [...deps]) or memoize
the returned objects), update getGenerationInstance's dependency array to
reference those memoized symbols (mediaGeneration, wordmarkGeneration,
symbolGeneration), and this will stabilize getGenerationInstance so the effect
that depends on it (the useEffect at the block ending with setInput) no longer
re-runs every render.
Member
There was a problem hiding this comment.
Only remove the functions in the dependency array. That way we can avoid re-renders.
Contributor
There was a problem hiding this comment.
@AndlerRL, understood! You're right—removing getGenerationInstance and setInput from the dependency array is a simpler and cleaner approach. This aligns with your established pattern for handling useEffect dependencies, where function references that don't represent changing values are excluded to prevent unnecessary re-renders.
The effect only needs to track the actual state values that determine when the draft should be inserted:
isWorkspaceActiveactiveGenerationScopeselectedSymbolStyle
Both getGenerationInstance and setInput are stable function references that don't need to be in the dependency array, and removing them will eliminate the unnecessary re-renders without adding memoization complexity.
✏️ Learnings added
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 601
File: apps/pro-web/components/routes/chat/prompt-form/index.tsx:333-339
Timestamp: 2026-02-24T16:33:41.127Z
Learning: In the pro-web app prompt-form component (apps/pro-web/components/routes/chat/prompt-form/index.tsx), exclude function references like getGenerationInstance and setInput from useEffect dependency arrays when they cause unnecessary re-renders. Only include the actual state values that determine when the effect should run.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T18:53:44.700Z
Learning: Applies to **/*.{tsx,jsx} : Minimize useEffect dependencies to prevent infinite loops and unnecessary re-renders
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 300
File: apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx:120-123
Timestamp: 2024-11-20T04:00:45.413Z
Learning: In React code, when reviewing `useEffect` dependencies, it's not always necessary to add functions like `handleThreadsChange` to the dependency array, as this may lead to unnecessary re-renders and performance issues. Only include dependencies that are essential and may change.
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-06T18:53:00.520Z
Learning: Applies to **/*.{jsx,tsx} : In React/State Management, minimize useEffect dependencies to prevent loops, use refs for non-re-render values, consider component lifecycle timing for DOM operations
Learnt from: Bran18
Repo: bitcashorg/masterbots PR: 376
File: apps/masterbots.ai/components/routes/chat/chat.tsx:92-102
Timestamp: 2025-02-17T16:51:19.102Z
Learning: For the Chat component in apps/masterbots.ai/components/routes/chat/chat.tsx, the developer prefers to verify potential optimizations through local testing before accepting suggestions, particularly for performance-related changes like useCallback usage in debounce implementations.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 340
File: apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx:191-195
Timestamp: 2025-01-16T20:48:20.108Z
Learning: In React's useEffect, it's acceptable to exclude function dependencies from the dependency array when:
1. The function only uses stable references (like setState functions)
2. Including the function could lead to unnecessary re-renders and performance issues
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 337
File: apps/masterbots.ai/components/routes/thread/user-thread-panel.tsx:195-205
Timestamp: 2025-01-14T06:20:03.578Z
Learning: React's useState setters (setState functions) maintain referential stability between renders and don't need to be included in useEffect dependency arrays. Include only the state values that the effect actually needs to track.
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-06T18:53:44.700Z
Learning: Applies to **/*.{tsx,jsx} : Use regular function calls on components instead of arrow functions to prevent unnecessary re-renders
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-06T18:53:00.520Z
Learning: Applies to **/*.{jsx,tsx} : Use regular function calls on components instead of arrow functions for event handlers and callbacks to prevent unnecessary re-renders
Learnt from: CR
Repo: bitcashorg/masterbots PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-06T18:52:26.701Z
Learning: Applies to **/*.{tsx,jsx} : When attaching event handlers to components, use regular function calls instead of arrow functions to prevent unnecessary re-renders and potential build errors from hoisting
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 0
File: :0-0
Timestamp: 2026-01-22T23:10:37.056Z
Learning: AndlerRL emphasizes Modular Development: avoid duplication by centralizing shared utilities/types, prefer small composable units, and keep flows simple and explicit (no cumbersome validations or over-engineering).
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 542
File: apps/web/components/layout/profile/layout.tsx:67-68
Timestamp: 2025-08-29T20:34:24.433Z
Learning: AndlerRL prefers skeleton loading states over conditional rendering for better UX when components are waiting for data to load. Components should show loading indicators rather than blank spaces.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:221-221
Timestamp: 2026-01-22T23:03:44.735Z
Learning: In TSX files (e.g., apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx), adopt a convention where variables that reference DOM elements are prefixed with '$' (e.g., $questionElement, $el) to quickly identify DOM refs. Apply this consistently across the codebase to improve readability and searchability; ensure the prefix is only used for actual DOM element references and not for non-DOM variables.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:40-40
Timestamp: 2026-01-22T23:03:49.088Z
Learning: In TypeScript React files, prefer using optional chaining with a nullish coalescing fallback (e.g., object?.param ?? fallback) over explicit type-guard helpers like isObject or asType when accessing nested properties. This keeps code simpler and more idiomatic. Ensure you still handle cases where intermediate properties may be undefined before applying the ?? fallback.
Learnt from: AndlerRL
Repo: bitcashorg/masterbots PR: 600
File: apps/pro-web/components/routes/chat/chat-list/message-pair-accordion.tsx:221-221
Timestamp: 2026-01-22T23:03:43.504Z
Learning: Enforce a DOM element naming convention in the pro-web app: in TSX files (e.g., apps/pro-web/**/*.tsx), prefix variables that hold DOM element references with a leading '$'. For example: const $questionElement = document.getElementById('question'); or const $list = document.querySelector('.list'); This helps quickly identify DOM refs and distinguish them from other variables. Apply this to variables obtained via document.getElementById, querySelector, etc., and ensure non-DOM variables are not prefixed.
310dcdd to
849422a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/pro-web/components/routes/chat/prompt-form/index.tsx`:
- Around line 593-597: The media submission guard currently checks activeTab ===
'media' and can call handleMediaPromptSubmit even when the workspace is
inactive; change the condition to use the existing boolean isWorkspaceMediaTab
(which is defined as isWorkspaceActive && activeTab === 'media') so media
submissions only run when both the workspace is active and the media tab is
selected, i.e., replace the activeTab === 'media' check with isWorkspaceMediaTab
where handleMediaPromptSubmit(trimmedInput) is invoked.
🧹 Nitpick comments (1)
apps/pro-web/components/routes/chat/prompt-form/index.tsx (1)
320-332: Nit: Use$elprefix for DOM element references.Per the project's DOM naming convention, variables holding DOM element references should be prefixed with
$.Proposed fix
setTimeout(() => { if (!inputRef.current) return - const el = inputRef.current - el.focus() + const $el = inputRef.current + $el.focus() // Move caret to end - el.setSelectionRange(draft.length, draft.length) + $el.setSelectionRange(draft.length, draft.length) // Scroll to bottom - el.scrollTop = el.scrollHeight + $el.scrollTop = $el.scrollHeight }, 0)Based on learnings: "In TSX files, adopt a convention where variables that reference DOM elements are prefixed with '$' (e.g., $questionElement, $el) to quickly identify DOM refs."
Comment on lines
+593
to
597
| // Media scope: generate or refine image | ||
| if (activeTab === 'media') { | ||
| await handleMediaPromptSubmit(trimmedInput) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Use isWorkspaceMediaTab instead of activeTab === 'media' to guard media submission.
Line 169 correctly defines isWorkspaceMediaTab = isWorkspaceActive && activeTab === 'media', but the submission guard on line 594 only checks activeTab === 'media'. If activeTab retains a stale 'media' value while the workspace is inactive, the regular chat submission path is bypassed in favor of handleMediaPromptSubmit.
Proposed fix
// Media scope: generate or refine image
- if (activeTab === 'media') {
+ if (isWorkspaceMediaTab) {
await handleMediaPromptSubmit(trimmedInput)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Media scope: generate or refine image | |
| if (activeTab === 'media') { | |
| await handleMediaPromptSubmit(trimmedInput) | |
| return | |
| } | |
| // Media scope: generate or refine image | |
| if (isWorkspaceMediaTab) { | |
| await handleMediaPromptSubmit(trimmedInput) | |
| return | |
| } |
🤖 Prompt for AI Agents
In `@apps/pro-web/components/routes/chat/prompt-form/index.tsx` around lines 593 -
597, The media submission guard currently checks activeTab === 'media' and can
call handleMediaPromptSubmit even when the workspace is inactive; change the
condition to use the existing boolean isWorkspaceMediaTab (which is defined as
isWorkspaceActive && activeTab === 'media') so media submissions only run when
both the workspace is active and the media tab is selected, i.e., replace the
activeTab === 'media' check with isWorkspaceMediaTab where
handleMediaPromptSubmit(trimmedInput) is invoked.
AndlerRL
approved these changes
Feb 24, 2026
Member
AndlerRL
left a comment
AndlerRL
left a comment
There was a problem hiding this comment.
LGTM! Revising deeply, this nails with the core intention for this new whole BrandKit feature. The steps and experience are smooth, I was able to go from vibe & tone to logo generation (including closing and opening again) with no issues—however, doing a thoughtful test, I discovered a set of flaws that are part of the happy paths which aren't a blocker to merge this PR, but key to work on those later on:
- When creating a logo symbol, there was no continuation of it: I asked to do a logo based on another draft logo that I had. It did a decent job at first iteration, but failed to continue it from the 2nd and I couldn't go back with iterations, but I did manually. The image references are colliding with continuation. The prompt has to improve and/or references should be removed once the first iteration is done, so the AI grab what it is mostly important, the new generated logo in addition with summarized context from the vibe & tone and symbol direction. With wordmark happens the same, on 2nd iteration the text is completely replaced by the initial text reference and results were average.
- When creating both logo based on a reference, fails to follow user intention: It is clear that a direction is given as user prompt, however, the way how the prompt is presented is not the most optimal. We should design a set of bullet that are sticky with the input bar to see the (suggested) options available for the logo symbol direction or prompt the user for the styles (like when creating a prompt template with prompt enhancer) with examples that align with their selection, so they are conscious of what direction we are giving to them and help them to do it. I could modify inline the directions, but I had to think and see what direction should I make without any suggestion that is beyond the logo style.
- When finishing the steps, is confusing where it takes: At the end of the steps, I see a successful state, however, I was incapable to see the BrandKit overview showing me my current status and CTAs to do (go back to vibe & tone, logo generation or see brand assets). When I close and open again, it will suggest me to create a new/import brand.
- Logo generator steps are aligned to the top: align them to the centre, so it can be centred on large screens. Layout looks sharp✨
🗒️ Major Feedback:
We need to review the prompts, most of the experience weren't good enough due the prompt that is injected into the nano banana pro and gpt-image-1 models when creating the logos. Once that standardized and we remove the old (non-related) references on continuous iterations, the experience would definitely improve.
Let's handle those in the next PRs. I will be creating issues with coderabbitai to track these and provide feedback too. The rest of feedbacks in this PR, please take them too in the new PRs. Thanks for this great feature! 🔥 Merging now.
Comment on lines
+111
to
+142
| // Validate logoAssets if provided | ||
| if (body.logoAssets !== undefined) { | ||
| if (typeof body.logoAssets !== 'object' || body.logoAssets === null) { | ||
| return NextResponse.json( | ||
| { error: 'Logo assets must be an object' }, | ||
| { status: 422 }, | ||
| ) | ||
| } | ||
|
|
||
| if (!body.logoAssets.symbol && !body.logoAssets.wordmark) { | ||
| return NextResponse.json( | ||
| { | ||
| error: | ||
| 'At least one logo asset (symbol or wordmark) must be provided', | ||
| }, | ||
| { status: 422 }, | ||
| ) | ||
| } | ||
|
|
||
| if (body.logoAssets.wordmark) { | ||
| if ( | ||
| !body.logoAssets.wordmark.name || | ||
| typeof body.logoAssets.wordmark.name !== 'string' || | ||
| !body.logoAssets.wordmark.name.trim() | ||
| ) { | ||
| return NextResponse.json( | ||
| { error: 'Wordmark name is required and must be non-empty' }, | ||
| { status: 422 }, | ||
| ) | ||
| } | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
It is better to add a verification similar as decodeBase64Content util to do this validation elsewhere needed.
Comment on lines
+334
to
+340
| }, [ | ||
| isWorkspaceActive, | ||
| activeGenerationScope, | ||
| selectedSymbolStyle, | ||
| getGenerationInstance, | ||
| setInput, | ||
| ]) |
Member
There was a problem hiding this comment.
Only remove the functions in the dependency array. That way we can avoid re-renders.
| const fullMarkdown = currentDocumentData?.content ?? '' | ||
| const sections = | ||
| currentDocumentData?.sections ?? | ||
| (fullMarkdown ? parseMarkdownSections(fullMarkdown) : undefined) |
Member
There was a problem hiding this comment.
Here I made a quick fix on impr/prompt-enhancer branch
|
|
||
| // Handle switch fonts | ||
| const handleSwitchFonts = () => { | ||
| if (selectedFonts.length === 2) { |
Member
There was a problem hiding this comment.
💡 PRO TIP: It is better to handle negative situations rather than positives. When doing conditions where the conditions met are the happy path. That shouldn't be always the case, since a function should be built focused on the happy path and handling the edge cases and/or negative conditions. For this function for example, it is easier to read/maintain:
const handleSwitchFonts = () => {
if (selectedFonts.length !== 2) return
setSelectedFonts([
{ family: selectedFonts[1].family, role: 'primary' },
{ family: selectedFonts[0].family, role: 'secondary' },
])
}See how we focus the core logic in the main function context window. Not priority to take though, for further additions consider this pattern.
| if (!wordmarkFontFamily && selectedFonts.length > 0) { | ||
| setWordmarkFontFamily(selectedFonts[0].family) | ||
| } | ||
| }, [wordmarkFontFamily, selectedFonts, setWordmarkFontFamily]) |
Member
There was a problem hiding this comment.
Keep array dependency as short as possible, mostly on useEffects. On next updates, keep in mind to remove the dispatch state functions into the array unless necessary; otherwise it may cause more re-renders.
| <div className="sticky top-0 z-10 bg-background/95 backdrop-blur supports-[backdrop-filter]:bg-background/80 flex flex-col items-center w-full gap-3 py-4"> | ||
| <div className="flex items-start justify-center w-full px-4 sm:px-6"> | ||
| <div className="flex items-start w-full max-w-6xl"> | ||
| {allSteps.map((step, displayIndex) => { |
Member
There was a problem hiding this comment.
💡 PRO TIP: Moving this away from the render cycle benefits the component performance, making it optimal on re-renders. While in some cases is OK to have it like it, it would depend on how many variables, you are adding into the render cycle. This case is better to have them separately, so it can optimize receiving the re-render new data.
Comment on lines
+97
to
+113
| /** | ||
| * Upload logo asset to GCS - symbol overload | ||
| */ | ||
| export async function uploadLogoAssetToGCS( | ||
| organizationId: string, | ||
| type: 'symbol', | ||
| input: SymbolLogoAssetInput, | ||
| ): Promise<SymbolLogoAsset> | ||
|
|
||
| /** | ||
| * Upload logo asset to GCS - wordmark overload | ||
| */ | ||
| export async function uploadLogoAssetToGCS( | ||
| organizationId: string, | ||
| type: 'wordmark', | ||
| input: WordmarkLogoAssetInput, | ||
| ): Promise<WordmarkLogoAsset> |
Member
There was a problem hiding this comment.
Duplicates! Please remove these.
Comment on lines
+759
to
+817
| // Shuffle selected palette order deterministically | ||
| const shuffleSelectedPaletteOrder = useCallback(() => { | ||
| if (!selectedPalette) return | ||
|
|
||
| const paletteName = selectedPalette.name | ||
| const baseColors = [ | ||
| selectedPalette.primary, | ||
| selectedPalette.accent, | ||
| selectedPalette.bg, | ||
| selectedPalette.text, | ||
| ] | ||
|
|
||
| // Generate all valid permutations | ||
| const allPermutations = generatePermutations(baseColors) | ||
|
|
||
| // Filter to valid combinations only | ||
| const validPermutations = allPermutations.filter((perm) => { | ||
| const [, , bg, text] = perm | ||
| // bg and text must have good contrast (WCAG AA) | ||
| return isValidTextCombination(bg, text) | ||
| }) | ||
|
|
||
| if (validPermutations.length === 0) { | ||
| console.warn('[shuffleSelectedPaletteOrder] No valid permutations found') | ||
| return | ||
| } | ||
|
|
||
| // Get current cursor for this palette (default to 0) | ||
| const currentCursor = shuffleState[paletteName] ?? 0 | ||
|
|
||
| // Advance to next valid permutation (cycle) | ||
| const nextCursor = (currentCursor + 1) % validPermutations.length | ||
| const nextPerm = validPermutations[nextCursor] | ||
|
|
||
| // Update shuffle state | ||
| setShuffleState((prev) => ({ ...prev, [paletteName]: nextCursor })) | ||
|
|
||
| // Update selectedPalette with new color assignments | ||
| setSelectedPalette({ | ||
| name: selectedPalette.name, | ||
| category: selectedPalette.category, | ||
| primary: nextPerm[0], | ||
| accent: nextPerm[1], | ||
| bg: nextPerm[2], | ||
| text: nextPerm[3], | ||
| }) | ||
|
|
||
| // Also update colorRoleItems to match the new palette order | ||
| // Keep selection state per position | ||
| setColorRoleItems((prev) => { | ||
| const selections = prev.map((item) => item.selected) | ||
| return [ | ||
| { role: 'primary', color: nextPerm[0], selected: selections[0] }, | ||
| { role: 'secondary', color: nextPerm[2], selected: selections[1] }, // bg maps to secondary | ||
| { role: 'accent', color: nextPerm[1], selected: selections[2] }, | ||
| { role: 'muted', color: nextPerm[3], selected: selections[3] }, // text maps to muted | ||
| ] | ||
| }) | ||
| }, [selectedPalette, shuffleState]) |
Member
There was a problem hiding this comment.
Good to dive deep and test edge cases in case they shuffle a lot or one or more colours are disabled
Member
|
@coderabbitai I recently reviewed this PR and we should be take care of the observations and final revision feedback made on this PR. I would like to track these feedback in a set of issues detailed with suggested actions to take. Consider the major feedback as the core of the improvements, since they are mostly the reason of these issues for UI/UX and functionality improvements:
|
This was referenced Feb 24, 2026
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 189628
🏁 Script executed: Length of output: 897
🏁 Script executed: Length of output: 346
These follow your preference for a consolidated tracker with clear P0/P1/P2 priorities. Would you like me to add labels (e.g., enhancement, UX, backend/frontend) or assign co-owners for each theme? I can also spin out separate micro-issues for any of the minor/nitpick inline comments you want tracked individually. ✏️ Learnings added
🧠 Learnings used |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎬 Video Demo
https://www.loom.com/share/ec78833eff904f8c9a440c49a08d475f
Summary by Sourcery
Introduce a multi-step AI-assisted logo generation flow integrated with the Brand Kit, including a completed wordmark style step, foundational direction/refinement/confirmation steps, and supporting typography and asset infrastructure.
New Features:
Enhancements:
Build:
Summary by CodeRabbit
New Features
Improvements