Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds opt-in, type-scoped generation (genTypes) end-to-end: UI for selecting content types, persisted settings, URL propagation, transport payload inclusion, handler usage, and API system-prompt generation that tailors and enforces allowed output types (note, quiz, flashcard, youtube). Changes
Sequence DiagramsequenceDiagram
actor User
participant HomeUI as HomePromptInput
participant Transport as WorkspaceRuntimeProvider
participant Handlers as AssistantPanel
participant API as Chat API
User->>HomeUI: Select genTypes / toggle auto
HomeUI->>HomeUI: Persist GenerationSettings (localStorage)
HomeUI->>HomeUI: Navigate/init with genTypes in URL
HomeUI->>Transport: Init/navigation contains genTypes
Transport->>Transport: Read genTypes from searchParams
Transport->>Transport: Include genTypes in transport payload
Transport->>Handlers: Initialize transport (contains genTypes)
Handlers->>Handlers: Read genTypes from searchParams
Handlers->>Handlers: Build type-aware message/prompt
Handlers->>API: POST messages + genTypes
API->>API: Parse genTypes, call getCreateFromSystemPrompt(messages, genTypes)
API-->>Handlers: Return generated content scoped to genTypes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 75abd9c in 23 seconds. Click for details.
- Reviewed
631lines of code in4files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_J3TUvQ9ZQ8Wo09I2
You can customize 
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/app/api/chat/route.ts">
<violation number="1" location="src/app/api/chat/route.ts:99">
P2: The non-greedy capture in the createFrom regex truncates topics that contain periods (e.g., "U.S. history" becomes "U"). Use a greedy capture with backtracking so the optional YouTube suffix still matches while preserving the full topic.</violation>
<violation number="2" location="src/app/api/chat/route.ts:247">
P2: `genTypes` splitting doesn’t trim whitespace, so values like "note, quiz" won’t match the expected type keys and the prompt may omit requested sections. Trim and filter the split values before building the Set.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@src/app/api/chat/route.ts`:
- Around line 128-131: The push strings for updateInstructions are using single
quotes so the ${topic} placeholder is not interpolated; change those three
string literals (the ones pushed when types.has('note'), types.has('flashcard'),
and types.has('quiz') in route.ts) to template literals using backticks so
${topic} is expanded (i.e., update the push calls that reference
updateNote/updateFlashcards/updateQuiz to use backtick strings including
${topic}).
- Around line 159-162: The prompt template in route.ts creates a numbering gap
because the hardcoded "4." assumes a conditional "3." injected by the
`restriction` variable (used alongside `updateInstructions.join('\n')`), so when
`restriction` is empty the list skips 3; fix this by building the instruction
lines programmatically instead of hardcoding indices — e.g., assemble an array
of items (using `updateInstructions` entries and conditionally pushing
`restriction` if present) and then map them to numbered strings
(`items.map((it,i)=>\`\${i+1}. \${it}\`)`) or always include a neutral item 3
when `genTypes`/`restriction` is missing; update the code that constructs the
prompt string (the template using `updateInstructions` and `restriction`) to use
this dynamic numbering approach so the sequence stays continuous.
In `@src/components/assistant-ui/AssistantPanel.tsx`:
- Around line 206-215: The prompt can be malformed when genTypes is set but
contains no recognized values because types/steps end up empty; adjust the logic
in AssistantPanel.tsx around genTypes/types/steps so you detect an empty steps
array before constructing prompt and provide a sensible fallback (e.g., populate
steps with a default instruction set or treat types as null to include all
steps), then build the prompt using the possibly-updated steps array; look for
the genTypes split, the types Set, the steps array and the prompt template to
apply the fix.
In `@src/components/home/HomePromptInput.tsx`:
- Around line 264-308: Duplicate placeholder construction should be
consolidated: add a helper like buildPlaceholderItems(effectiveTypes, startY,
variant?) that returns the array of placeholders with consistent fields (id,
type, name, subtitle, color, layout, lastSource, data) and deterministic
per-type colors (e.g., map type->color for 'note','quiz','flashcard') and
correct heights/layout logic; use this helper to replace both the uploads and
no-uploads blocks in HomePromptInput.tsx so every placeholder includes
lastSource: 'user' and uses the same color assignment and layout rules (refer to
the existing placeholder creation objects and variables currentY/totalUploadY,
pdfItems, and allItems to integrate the returned placeholders).
- Around line 618-689: The toggle buttons for content types (rendered in the
ALL_CONTENT_TYPES.map using toggleGenType and reading genSettings.types) and the
Auto toggle (which calls setGenSettings and reads genSettings.auto) are missing
accessibility attributes; update each toggle button element to include
role="switch", aria-checked set to the boolean active state (for type toggles:
genSettings.types.includes(key); for Auto: genSettings.auto), and add an
appropriate aria-label (e.g., `${label} toggle` for type toggles and "Auto
toggle" for Auto) so screen readers announce the control and its state; keep the
existing onClick handlers (toggleGenType / setGenSettings) intact.
🧹 Nitpick comments (1)
src/app/api/chat/route.ts (1)
247-248: No server-side validation ofgenTypesvalues.
body.genTypesis split and passed directly without validating that each element is a recognized type. While not a security risk (values are only used in prompt construction), invalid types silently produce an incomplete prompt. Consider filtering to known types.🛡️ Suggested validation
- const genTypes: string[] | null = body.genTypes ? body.genTypes.split(',') : null; + const VALID_GEN_TYPES = new Set(['note', 'quiz', 'flashcard', 'youtube']); + const genTypes: string[] | null = body.genTypes + ? body.genTypes.split(',').filter((t: string) => VALID_GEN_TYPES.has(t)) + : null; + // Treat empty filtered array as null (all types) + const effectiveGenTypes = genTypes && genTypes.length > 0 ? genTypes : null;
Greptile OverviewGreptile SummaryThis PR adds user-selectable “generation types” (notes/quizzes/flashcards/YouTube) with persistence, threads the selected types via The main integration points are:
One blocking issue remains: YouTube is a selectable type but no YouTube placeholder item is created in the new placeholder builder, which can produce an initial workspace state that cannot satisfy the type-scoped instructions. Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Home as HomePromptInput
participant Router
participant WS as WorkspacePage
participant Panel as AssistantPanel
participant Runtime as WorkspaceRuntimeProvider
participant API as ChatRoute
User->>Home: Submit prompt + select types
Home->>Router: push /workspace/:slug with genTypes
Router->>WS: Navigate to workspace
WS->>Panel: Mount auto-init handlers
Panel->>Panel: Build type-aware prompt from genTypes
Panel->>Runtime: composer.send(prompt)
Runtime->>API: POST /api/chat (body.genTypes when auto-init)
API->>API: Parse/validate genTypes
API->>API: Inject createFrom system prompt
API-->>Runtime: Stream response
Panel->>Router: replace() remove genTypes/createFrom/action
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/app/api/chat/route.ts
Outdated
| if (types.has('note')) updateInstructions.push(' - Use `updateNote` with noteName "Update me" and a descriptive `title` (e.g. "${topic} Notes")'); | ||
| if (types.has('flashcard')) updateInstructions.push(' - Use `updateFlashcards` with deckName "Update me" and a descriptive `title` (e.g. "${topic} Flashcards")'); | ||
| if (types.has('quiz')) updateInstructions.push(' - Use `updateQuiz` with quizName "Update me" and a descriptive `title` (e.g. "${topic} Quiz")'); | ||
|
|
There was a problem hiding this comment.
Template string not interpolated
The type-specific updateInstructions are built with single-quoted strings containing ${topic} (e.g. "${topic} Notes"), so ${topic} will be sent literally in the system prompt rather than being substituted. This breaks the intended “descriptive title” guidance for create-from initialization.
| if (types.has('note')) updateInstructions.push(' - Use `updateNote` with noteName "Update me" and a descriptive `title` (e.g. "${topic} Notes")'); | |
| if (types.has('flashcard')) updateInstructions.push(' - Use `updateFlashcards` with deckName "Update me" and a descriptive `title` (e.g. "${topic} Flashcards")'); | |
| if (types.has('quiz')) updateInstructions.push(' - Use `updateQuiz` with quizName "Update me" and a descriptive `title` (e.g. "${topic} Quiz")'); | |
| if (types.has('note')) updateInstructions.push(` - Use \`updateNote\` with noteName "Update me" and a descriptive \`title\` (e.g. "${topic} Notes")`); | |
| if (types.has('flashcard')) updateInstructions.push(` - Use \`updateFlashcards\` with deckName "Update me" and a descriptive \`title\` (e.g. "${topic} Flashcards")`); | |
| if (types.has('quiz')) updateInstructions.push(` - Use \`updateQuiz\` with quizName "Update me" and a descriptive \`title\` (e.g. "${topic} Quiz")`); |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 464ab35 in 18 seconds. Click for details.
- Reviewed
279lines of code in3files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_FRCrkpEyIBMIy0hv
You can customize
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/app/api/chat/route.ts">
<violation number="1" location="src/app/api/chat/route.ts:99">
P2: The greedy capture in the create-from regex causes the topic to include the optional YouTube sentence, so generated prompts will treat that suffix as part of the topic. Use a non-greedy capture to stop at the first period.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Regex to detect createFrom auto-generated prompts | ||
| const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+)\. Only add one quality YouTube video\.$/; | ||
| // Regex to detect createFrom auto-generated prompts (YouTube suffix is optional for custom type selections) | ||
| const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+)\.(?:\s*Only add one quality YouTube video\.)?$/; |
There was a problem hiding this comment.
P2: The greedy capture in the create-from regex causes the topic to include the optional YouTube sentence, so generated prompts will treat that suffix as part of the topic. Use a non-greedy capture to stop at the first period.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/app/api/chat/route.ts, line 99:
<comment>The greedy capture in the create-from regex causes the topic to include the optional YouTube sentence, so generated prompts will treat that suffix as part of the topic. Use a non-greedy capture to stop at the first period.</comment>
<file context>
@@ -96,7 +96,7 @@ function getSelectedCardsContext(body: any): string {
// Regex to detect createFrom auto-generated prompts (YouTube suffix is optional for custom type selections)
-const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+?)\.(?:\s*Only add one quality YouTube video\.)?$/;
+const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+)\.(?:\s*Only add one quality YouTube video\.)?$/;
/**
</file context>
| const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+)\.(?:\s*Only add one quality YouTube video\.)?$/; | |
| const CREATE_FROM_REGEX = /^Update the preexisting contents of this workspace to be about (.+?)\.(?:\s*Only add one quality YouTube video\.)?$/; |
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/app/api/chat/route.ts`:
- Around line 125-152: The "Update ONLY these workspace items" block is emitted
even when updateInstructions is empty (e.g., genTypes=['youtube']), producing an
empty/confusing instruction; update the logic in src/app/api/chat/route.ts
around the variables genTypes/types/updateInstructions and the instructions.push
call that uses instrNum to only add that numbered "Update ONLY..." entry when
updateInstructions.length > 0 (or alternatively add a YouTube-specific
updateInstructions entry when types.has('youtube')), so that the instructions
array never contains an empty update list.
🧹 Nitpick comments (4)
src/app/api/chat/route.ts (1)
249-250: Consider validating parsedgenTypesbefore use.If
body.genTypesis a truthy but degenerate string (e.g.",,") the pipeline produces an empty array, which passes the truthy check on line 125 and yields a prompt with no update instructions and the "Do NOT create other types" restriction — effectively telling the LLM to do nothing.A simple guard normalises this to the default behaviour:
Suggested fix
- const genTypes: string[] | null = body.genTypes ? body.genTypes.split(',').map((t: string) => t.trim()).filter(Boolean) : null; + const genTypesParsed: string[] = body.genTypes ? body.genTypes.split(',').map((t: string) => t.trim()).filter(Boolean) : []; + const genTypes: string[] | null = genTypesParsed.length > 0 ? genTypesParsed : null;src/components/home/HomePromptInput.tsx (3)
50-96: Good extraction ofbuildPlaceholderItems— addresses the prior duplication concern.The shared helper consolidates the two code paths. Layout logic (side-by-side quiz + flashcard, full-width note) is sound.
One nit: the return type is
any[]. If the workspace item shape is defined elsewhere (e.g., intypes.ts), a concrete type would prevent silent shape drift.
98-111:loadGenSettingsdoesn't validate individual type values.If
localStoragecontains stale or tampered values (e.g.types: ["note", "podcast"]), the invalid entries propagate through toeffectiveTypesand ultimately to thegenTypesURL param / API payload, where they'll be silently ignored byroute.tsbut still occupy prompt space.A quick filter against
ALL_CONTENT_TYPESkeys would tighten this:Suggested fix
+ const VALID_TYPES = new Set<string>(ALL_CONTENT_TYPES.map(t => t.key)); + function loadGenSettings(): GenerationSettings { if (typeof window === 'undefined') return DEFAULT_GEN_SETTINGS; try { const stored = localStorage.getItem('thinkex-gen-settings'); if (stored) { const parsed = JSON.parse(stored); return { auto: typeof parsed.auto === 'boolean' ? parsed.auto : true, - types: Array.isArray(parsed.types) ? parsed.types : DEFAULT_GEN_SETTINGS.types, + types: Array.isArray(parsed.types) + ? parsed.types.filter((t: string) => VALID_TYPES.has(t)) + : DEFAULT_GEN_SETTINGS.types, }; } } catch {} return DEFAULT_GEN_SETTINGS; }
163-168: localStorage write on every settings change is fine, but the emptycatchsilently swallows quota errors.If the user hits a storage quota limit, the settings will appear saved (state updates) but won't persist across sessions. A minimal
console.warninside the catch would help debug this in the field without affecting UX.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed eed1885 in 29 seconds. Click for details.
- Reviewed
15lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_uBgHNNsom8Zpq2sQ
You can customize
| function buildPlaceholderItems( | ||
| effectiveTypes: Set<GenContentType>, | ||
| startY: number, | ||
| heights: { note: number; flashcard: number }, | ||
| ): any[] { | ||
| const placeholders: any[] = []; | ||
| let currentY = startY; | ||
|
|
||
| if (effectiveTypes.has('note')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'note', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#10B981', | ||
| layout: { x: 0, y: currentY, w: 4, h: heights.note }, | ||
| data: { blockContent: [], field1: '' }, | ||
| }); | ||
| currentY += heights.note; | ||
| } | ||
|
|
||
| if (effectiveTypes.has('quiz')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'quiz', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#F59E0B', | ||
| layout: { x: 0, y: currentY, w: 2, h: 13 }, | ||
| data: { questions: [] }, | ||
| }); | ||
| } | ||
|
|
||
| if (effectiveTypes.has('flashcard')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'flashcard', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#EC4899', | ||
| layout: { x: effectiveTypes.has('quiz') ? 2 : 0, y: currentY, w: 2, h: heights.flashcard }, | ||
| data: { cards: [] }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Invalid flashcard placeholder shape
buildPlaceholderItems() creates a flashcard item with data: { cards: [] }, but the workspace state expects flashcards to include additional fields like currentIndex and typically card entries with id/front/back/frontBlocks/backBlocks (see defaultDataFor("flashcard") in src/lib/workspace-state/item-helpers.ts:10-28). Creating placeholders with the wrong shape can break flashcard rendering/updating when the workspace loads. You should construct flashcard placeholder data using the same shape as defaultDataFor("flashcard") (or call that helper) so the created workspace state is valid.
|
@greptile |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 147a6c1 in 34 seconds. Click for details.
- Reviewed
21lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_J26PCFo43ifSyYcv
You can customize
| function loadGenSettings(): GenerationSettings { | ||
| if (typeof window === 'undefined') return DEFAULT_GEN_SETTINGS; | ||
| try { | ||
| const stored = localStorage.getItem('thinkex-gen-settings'); | ||
| if (stored) { | ||
| const parsed = JSON.parse(stored); | ||
| return { | ||
| auto: typeof parsed.auto === 'boolean' ? parsed.auto : true, | ||
| types: Array.isArray(parsed.types) ? parsed.types : DEFAULT_GEN_SETTINGS.types, | ||
| }; | ||
| } | ||
| } catch {} | ||
| return DEFAULT_GEN_SETTINGS; |
There was a problem hiding this comment.
Unvalidated localStorage shape
loadGenSettings() accepts any array for parsed.types and persists it back to localStorage. If types contains values outside GenContentType (e.g. from a previous version or manual editing), the UI can enter a state where auto is off but steps/system prompt end up empty (because nothing matches), which then forces the fallback-to-all behavior later and effectively ignores the user’s selection. Consider filtering parsed.types to the allowed keys (note|quiz|flashcard|youtube) when loading, so “custom” mode can’t be activated with an invalid/empty effective set.
| // Build type-aware prompt: only include steps for selected types | ||
| const types = genTypes ? new Set(genTypes.split(',')) : null; | ||
| const steps: string[] = []; | ||
| let n = 1; | ||
| if (!types || types.has('note')) steps.push(`${n++}. Update the note with a comprehensive summary`); | ||
| if (!types || types.has('quiz')) steps.push(`${n++}. Update the quiz with 5-10 relevant questions`); | ||
| if (!types || types.has('flashcard')) steps.push(`${n++}. Update the flashcards with key terms and concepts`); | ||
| if (!types || types.has('youtube')) steps.push(`${n++}. Search and add one relevant YouTube video if possible`); | ||
|
|
||
| // If genTypes was set but contained no recognized values, fall back to all types | ||
| if (steps.length === 0) { | ||
| steps.push('1. Update the note with a comprehensive summary'); | ||
| steps.push('2. Update the quiz with 5-10 relevant questions'); | ||
| steps.push('3. Update the flashcards with key terms and concepts'); | ||
| steps.push('4. Search and add one relevant YouTube video if possible'); | ||
| } | ||
|
|
||
| const prompt = `First, process any PDF files in this workspace.\n\nThen, using the content:\n${steps.join('\n')}`; |
There was a problem hiding this comment.
genTypes parsing misses whitespace
genTypes.split(',') isn’t trimmed before being put into the Set. A query like ?genTypes=note, quiz will produce ' quiz' which won’t match types.has('quiz'), dropping steps unexpectedly and potentially triggering the “fallback to all” path. Trimming each entry (and filtering empties) when parsing would keep the handler consistent with the server-side parsing in route.ts.
Additional Comments (1)
|
|
@greptile |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed d208c3b in 32 seconds. Click for details.
- Reviewed
44lines of code in2files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_iCpPOdJbynRmArbj
You can customize
| const selectedActions = useUIStore((state) => state.selectedActions); | ||
| const replySelections = useUIStore(useShallow((state) => state.replySelections)); | ||
| const searchParams = useSearchParams(); | ||
| const genTypes = searchParams.get("genTypes"); | ||
| const { data: session } = useSession(); |
There was a problem hiding this comment.
Leaking URL genTypes
WorkspaceRuntimeProvider always reads genTypes from the URL and sends it in every /api/chat request body (body.genTypes), but AssistantPanel deletes genTypes from the URL only when it auto-sends createFrom/action prompts. If the user opens a workspace URL containing genTypes without those params (or after manual navigation), genTypes will persist and unintentionally constrain all future chats (server-side route.ts will restrict create-from instructions based on it). Consider scoping genTypes to only the auto-init flows (e.g., only attach it when createFrom/action=generate_study_materials is present, or remove it from the URL on load even when no auto-send triggers).
| if (effectiveTypes.has('flashcard')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'flashcard', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#EC4899', | ||
| layout: { x: effectiveTypes.has('quiz') ? 2 : 0, y: currentY, w: 2, h: heights.flashcard }, | ||
| data: defaultDataFor('flashcard'), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing YouTube placeholder
The new generation settings include youtube, and effectiveTypes can contain it, but buildPlaceholderItems() never creates a placeholder item for YouTube. When users select only YouTube (or include YouTube alongside others), the initial workspace state will not contain any YouTube item for the assistant to update, so the type-aware prompts/instructions won’t be satisfiable. If YouTube is meant to be a workspace item type, add a corresponding placeholder here (using the same shape as the rest of the app expects for a YouTube card).
|
@greptile |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c7883af in 8 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_PAjebhX62HQM3v9R
You can customize
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/app/api/chat/route.ts`:
- Around line 167-171: The prompt currently inserts the "QUALITY GUIDELINES FOR
CONTENT:" header even when qualityGuidelines is empty (e.g. genTypes ===
['youtube']); update the prompt assembly in route.ts so the header and joined
qualityGuidelines are only included when qualityGuidelines.length > 0 (or
truthy), otherwise omit that entire section; locate the variables
qualityGuidelines and youtubeSection and conditionally concatenate the header
plus qualityGuidelines.join(...) only when there are guidelines to show.
- Around line 251-252: Validate and sanitize body.genTypes before passing to
getCreateFromSystemPrompt by applying an allowlist of known generation types:
split and trim as you already do for genTypes, then filter each token against
the server-side allowed set (e.g., allowedGenTypes or the same set used inside
getCreateFromSystemPrompt, using a consistent normalization like toLowerCase()),
discard unknown values, and set genTypes to null if the resulting array is
empty; update the use-site where genTypes is declared and where
getCreateFromSystemPrompt(cleanedMessages, genTypes) is called so only
validated/allowed types reach getCreateFromSystemPrompt.
In `@src/components/home/HomePromptInput.tsx`:
- Around line 615-646: The content-type toggle buttons remain keyboard-focusable
when Auto mode is on because only the wrapper uses pointer-events-none; update
the button rendering inside the ALL_CONTENT_TYPES map (where toggleGenType is
used and active is computed from genSettings.types) to set
disabled={genSettings.auto} (or aria-disabled="true") and
tabIndex={genSettings.auto ? -1 : 0}, and move/adjust visual disabled styling to
rely on the :disabled pseudo-class (or keep pointer-events-none for pointer
visuals) so the toggles are both non-interactive and removed from keyboard focus
when genSettings.auto is true.
- Around line 333-353: The no-uploads custom-settings branch in
HomePromptInput.tsx builds placeholders via buildPlaceholderItems but doesn’t
set lastSource, causing inconsistency with the uploads path; update the branch
where placeholders are assigned (the isCustom block that sets initialState and
itemsCreated) to attach lastSource: 'user' to each placeholder (e.g., map over
placeholders before setting initialState) so initialState.items matches the
uploads path shape and downstream code relying on lastSource sees it.
🧹 Nitpick comments (2)
src/components/home/HomePromptInput.tsx (2)
286-293: Edge case: all types toggled off with auto disabled produces default-all behavior — by design?When
genSettings.autoisfalseandgenSettings.typesis empty,effectiveTypesfalls back to all four types. The popover'sonOpenChange(Line 588) snapsautoback totrueon close if types are empty, but there's a window where submission could happen while the popover is still open with no types selected.This is likely a rare edge case with a safe fallback (all types), just flagging for awareness.
166-171: Silent catch on localStorage write is fine, but consider a brief comment.Empty
catch {}blocks can look unintentional. A one-line comment like// localStorage may be unavailable (e.g. private browsing)would clarify intent for future readers. Very minor nit.
| const genTypes: string[] | null = body.genTypes ? body.genTypes.split(',').map((t: string) => t.trim()).filter(Boolean) : null; | ||
| const createFromPrompt = getCreateFromSystemPrompt(cleanedMessages, genTypes); |
There was a problem hiding this comment.
genTypes values are not validated against the known set of types.
body.genTypes comes from the client and is split/trimmed but never validated. Arbitrary strings pass through to getCreateFromSystemPrompt, where they silently miss every types.has(...) check, producing a prompt with empty instructions and guidelines sections. Consider filtering against an allowlist server-side:
Proposed fix
- const genTypes: string[] | null = body.genTypes ? body.genTypes.split(',').map((t: string) => t.trim()).filter(Boolean) : null;
+ const ALLOWED_GEN_TYPES = new Set(['note', 'quiz', 'flashcard', 'youtube']);
+ const genTypes: string[] | null = body.genTypes
+ ? body.genTypes.split(',').map((t: string) => t.trim()).filter((t: string) => ALLOWED_GEN_TYPES.has(t))
+ : null;
+ // Fall back to null if filtering removed all entries
+ const validGenTypes = genTypes && genTypes.length > 0 ? genTypes : null;🤖 Prompt for AI Agents
In `@src/app/api/chat/route.ts` around lines 251 - 252, Validate and sanitize
body.genTypes before passing to getCreateFromSystemPrompt by applying an
allowlist of known generation types: split and trim as you already do for
genTypes, then filter each token against the server-side allowed set (e.g.,
allowedGenTypes or the same set used inside getCreateFromSystemPrompt, using a
consistent normalization like toLowerCase()), discard unknown values, and set
genTypes to null if the resulting array is empty; update the use-site where
genTypes is declared and where getCreateFromSystemPrompt(cleanedMessages,
genTypes) is called so only validated/allowed types reach
getCreateFromSystemPrompt.
| // For no-uploads path with custom settings, build a custom initial state | ||
| // instead of using the getting_started template | ||
| let template: "blank" | "getting_started"; | ||
| if (hasUploads) { | ||
| template = "blank"; | ||
| } else if (isCustom) { | ||
| template = "blank"; | ||
| const placeholders = buildPlaceholderItems(effectiveTypes, 0, { note: 9, flashcard: 9 }); | ||
|
|
||
| if (placeholders.length > 0) { | ||
| initialState = { | ||
| workspaceId: '', | ||
| globalTitle: '', | ||
| globalDescription: '', | ||
| items: placeholders, | ||
| itemsCreated: placeholders.length, | ||
| }; | ||
| } | ||
| } else { | ||
| template = "getting_started"; | ||
| } |
There was a problem hiding this comment.
No-uploads placeholders are missing lastSource: 'user'.
The uploads path (Line 320) adds lastSource: 'user' to each placeholder, but the no-uploads custom-settings path (Line 340) does not. If downstream code relies on lastSource, this inconsistency could cause issues.
Proposed fix
- const placeholders = buildPlaceholderItems(effectiveTypes, 0, { note: 9, flashcard: 9 });
+ const placeholders = buildPlaceholderItems(effectiveTypes, 0, { note: 9, flashcard: 9 })
+ .map(item => ({ ...item, lastSource: 'user' as const }));🤖 Prompt for AI Agents
In `@src/components/home/HomePromptInput.tsx` around lines 333 - 353, The
no-uploads custom-settings branch in HomePromptInput.tsx builds placeholders via
buildPlaceholderItems but doesn’t set lastSource, causing inconsistency with the
uploads path; update the branch where placeholders are assigned (the isCustom
block that sets initialState and itemsCreated) to attach lastSource: 'user' to
each placeholder (e.g., map over placeholders before setting initialState) so
initialState.items matches the uploads path shape and downstream code relying on
lastSource sees it.
| <div className={cn("space-y-2", genSettings.auto && "opacity-40 pointer-events-none")}> | ||
| {ALL_CONTENT_TYPES.map(({ key, label }) => { | ||
| const active = genSettings.types.includes(key); | ||
| return ( | ||
| <button | ||
| key={key} | ||
| type="button" | ||
| role="switch" | ||
| aria-checked={active} | ||
| aria-label={`Toggle ${label}`} | ||
| className="flex items-center justify-between w-full" | ||
| onClick={() => toggleGenType(key)} | ||
| > | ||
| <span className="text-xs text-foreground">{label}</span> | ||
| <span | ||
| aria-hidden="true" | ||
| className={cn( | ||
| "relative inline-flex h-4 w-7 shrink-0 cursor-pointer rounded-full transition-colors", | ||
| active ? "bg-primary" : "bg-muted-foreground/25" | ||
| )} | ||
| > | ||
| <span | ||
| className={cn( | ||
| "pointer-events-none inline-block h-3 w-3 rounded-full bg-background shadow-sm ring-0 transition-transform mt-0.5", | ||
| active ? "translate-x-[14px]" : "translate-x-0.5" | ||
| )} | ||
| /> | ||
| </span> | ||
| </button> | ||
| ); | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
Content-type toggles remain keyboard-focusable when disabled by Auto mode.
pointer-events-none only blocks pointer interactions. Users can still Tab into the toggles and activate them with Space/Enter while they appear disabled. Consider adding tabIndex={-1} and disabled (or aria-disabled="true") when auto mode is on.
Proposed fix
<button
key={key}
type="button"
role="switch"
aria-checked={active}
aria-label={`Toggle ${label}`}
+ disabled={genSettings.auto}
className="flex items-center justify-between w-full"
onClick={() => toggleGenType(key)}
>Then replace the wrapper pointer-events-none approach with styling via the :disabled pseudo-class or keep both for visual consistency.
📝 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.
| <div className={cn("space-y-2", genSettings.auto && "opacity-40 pointer-events-none")}> | |
| {ALL_CONTENT_TYPES.map(({ key, label }) => { | |
| const active = genSettings.types.includes(key); | |
| return ( | |
| <button | |
| key={key} | |
| type="button" | |
| role="switch" | |
| aria-checked={active} | |
| aria-label={`Toggle ${label}`} | |
| className="flex items-center justify-between w-full" | |
| onClick={() => toggleGenType(key)} | |
| > | |
| <span className="text-xs text-foreground">{label}</span> | |
| <span | |
| aria-hidden="true" | |
| className={cn( | |
| "relative inline-flex h-4 w-7 shrink-0 cursor-pointer rounded-full transition-colors", | |
| active ? "bg-primary" : "bg-muted-foreground/25" | |
| )} | |
| > | |
| <span | |
| className={cn( | |
| "pointer-events-none inline-block h-3 w-3 rounded-full bg-background shadow-sm ring-0 transition-transform mt-0.5", | |
| active ? "translate-x-[14px]" : "translate-x-0.5" | |
| )} | |
| /> | |
| </span> | |
| </button> | |
| ); | |
| })} | |
| </div> | |
| <div className={cn("space-y-2", genSettings.auto && "opacity-40 pointer-events-none")}> | |
| {ALL_CONTENT_TYPES.map(({ key, label }) => { | |
| const active = genSettings.types.includes(key); | |
| return ( | |
| <button | |
| key={key} | |
| type="button" | |
| role="switch" | |
| aria-checked={active} | |
| aria-label={`Toggle ${label}`} | |
| disabled={genSettings.auto} | |
| className="flex items-center justify-between w-full" | |
| onClick={() => toggleGenType(key)} | |
| > | |
| <span className="text-xs text-foreground">{label}</span> | |
| <span | |
| aria-hidden="true" | |
| className={cn( | |
| "relative inline-flex h-4 w-7 shrink-0 cursor-pointer rounded-full transition-colors", | |
| active ? "bg-primary" : "bg-muted-foreground/25" | |
| )} | |
| > | |
| <span | |
| className={cn( | |
| "pointer-events-none inline-block h-3 w-3 rounded-full bg-background shadow-sm ring-0 transition-transform mt-0.5", | |
| active ? "translate-x-[14px]" : "translate-x-0.5" | |
| )} | |
| /> | |
| </span> | |
| </button> | |
| ); | |
| })} | |
| </div> |
🤖 Prompt for AI Agents
In `@src/components/home/HomePromptInput.tsx` around lines 615 - 646, The
content-type toggle buttons remain keyboard-focusable when Auto mode is on
because only the wrapper uses pointer-events-none; update the button rendering
inside the ALL_CONTENT_TYPES map (where toggleGenType is used and active is
computed from genSettings.types) to set disabled={genSettings.auto} (or
aria-disabled="true") and tabIndex={genSettings.auto ? -1 : 0}, and move/adjust
visual disabled styling to rely on the :disabled pseudo-class (or keep
pointer-events-none for pointer visuals) so the toggles are both non-interactive
and removed from keyboard focus when genSettings.auto is true.
| const topic = match[1]; | ||
| const types = genTypes ? new Set(genTypes) : new Set(['note', 'quiz', 'flashcard', 'youtube']); | ||
|
|
||
| // Build type-specific update instructions — always include title param | ||
| const updateInstructions: string[] = []; |
There was a problem hiding this comment.
Invalid genTypes yields no-op
getCreateFromSystemPrompt() treats any non-null genTypes as authoritative (const types = genTypes ? new Set(genTypes) : ...). If body.genTypes is present but contains no recognized values (e.g. genTypes=foo,bar or an empty/filtered client value), updateInstructions becomes empty and the prompt still adds the “Do NOT create or update any other item types” restriction, which can leave the model with no valid update actions to take during create-from initialization. Consider validating/filtering genTypes to the allowed set and falling back to default types when the recognized set is empty.
| function buildPlaceholderItems( | ||
| effectiveTypes: Set<GenContentType>, | ||
| startY: number, | ||
| heights: { note: number; flashcard: number }, | ||
| ): any[] { | ||
| const placeholders: any[] = []; | ||
| let currentY = startY; | ||
|
|
||
| if (effectiveTypes.has('note')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'note', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#10B981', | ||
| layout: { x: 0, y: currentY, w: 4, h: heights.note }, | ||
| data: { blockContent: [], field1: '' }, | ||
| }); | ||
| currentY += heights.note; | ||
| } | ||
|
|
||
| if (effectiveTypes.has('quiz')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'quiz', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#F59E0B', | ||
| layout: { x: 0, y: currentY, w: 2, h: 13 }, | ||
| data: { questions: [] }, | ||
| }); | ||
| } | ||
|
|
||
| if (effectiveTypes.has('flashcard')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'flashcard', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#EC4899', | ||
| layout: { x: effectiveTypes.has('quiz') ? 2 : 0, y: currentY, w: 2, h: heights.flashcard }, | ||
| data: defaultDataFor('flashcard'), | ||
| }); | ||
| } | ||
|
|
||
| return placeholders; | ||
| } |
There was a problem hiding this comment.
YouTube selection creates empty workspace
buildPlaceholderItems() never creates a placeholder for youtube, but GenContentType/settings allow selecting it and effectiveTypes can be {'youtube'}. In the no-uploads + custom path, this makes placeholders.length === 0, so you create a template="blank" workspace with initialState unset, yet still append genTypes=youtube and trigger the auto-init prompt. That leaves the assistant with no workspace item to update for the requested type. This also affects the uploads path (no YouTube placeholder ever exists). Either add a YouTube placeholder item type (matching what the workspace state expects) or disallow youtube as a selectable type until it maps to a real workspace item.
Also appears in the no-uploads custom path at handleSubmit where initialState is only set when placeholders.length > 0 (src/components/home/HomePromptInput.tsx:333-353).
|
@greptile |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f1cc7e5 in 16 seconds. Click for details.
- Reviewed
35lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_5bY7YnyFTtnBNiB2
You can customize
|
| if (effectiveTypes.has('quiz')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'quiz', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#F59E0B', | ||
| layout: { x: 0, y: currentY, w: 2, h: 13 }, | ||
| data: { questions: [] }, | ||
| }); | ||
| } | ||
|
|
||
| if (effectiveTypes.has('flashcard')) { | ||
| placeholders.push({ | ||
| id: crypto.randomUUID(), | ||
| type: 'flashcard', | ||
| name: 'Update me', | ||
| subtitle: '', | ||
| color: '#EC4899', | ||
| layout: { x: effectiveTypes.has('quiz') ? 2 : 0, y: currentY, w: 2, h: heights.flashcard }, | ||
| data: defaultDataFor('flashcard'), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing YouTube placeholder
GenContentType/settings allow selecting youtube, but buildPlaceholderItems() never creates a type: "youtube" item. If a user selects only YouTube (or any set that includes YouTube but excludes note/quiz/flashcard), the created workspace initialState.items can end up without any YouTube card to update, while AssistantPanel/route.ts prompts can still instruct adding/updating a YouTube video. This will make the auto-init flows unable to satisfy the requested type selection. Consider adding a YouTube placeholder using the existing defaultDataFor("youtube") shape.
| // For no-uploads path with custom settings, build a custom initial state | ||
| // instead of using the getting_started template | ||
| let template: "blank" | "getting_started"; | ||
| if (hasUploads) { | ||
| template = "blank"; | ||
| } else if (isCustom) { | ||
| template = "blank"; | ||
| const placeholders = buildPlaceholderItems(effectiveTypes, 0, { note: 9, flashcard: 9 }); | ||
|
|
||
| if (placeholders.length > 0) { | ||
| initialState = { | ||
| workspaceId: '', | ||
| globalTitle: '', | ||
| globalDescription: '', | ||
| items: placeholders, | ||
| itemsCreated: placeholders.length, | ||
| }; | ||
| } | ||
| } else { | ||
| template = "getting_started"; | ||
| } |
There was a problem hiding this comment.
Blank template without state
In the no-uploads + custom-types path you set template = "blank" but only set initialState when placeholders.length > 0. If the user selects only youtube (or any selection that yields zero placeholders), this creates a blank workspace with no items, yet you still append genTypes and trigger the auto-init prompt. That leaves the assistant with nothing to update for the selected types. This should either create the corresponding placeholder item(s) for the selected types, or avoid using template="blank" when initialState would be empty.
| // Build type-aware prompt: only include steps for selected types | ||
| const types = genTypes ? new Set(genTypes.split(',').map(s => s.trim()).filter(Boolean)) : null; | ||
| const steps: string[] = []; | ||
| let n = 1; | ||
| if (!types || types.has('note')) steps.push(`${n++}. Update the note with a comprehensive summary`); | ||
| if (!types || types.has('quiz')) steps.push(`${n++}. Update the quiz with 5-10 relevant questions`); | ||
| if (!types || types.has('flashcard')) steps.push(`${n++}. Update the flashcards with key terms and concepts`); | ||
| if (!types || types.has('youtube')) steps.push(`${n++}. Search and add one relevant YouTube video if possible`); | ||
|
|
||
| // If genTypes was set but contained no recognized values, fall back to all types | ||
| if (steps.length === 0) { | ||
| steps.push('1. Update the note with a comprehensive summary'); | ||
| steps.push('2. Update the quiz with 5-10 relevant questions'); | ||
| steps.push('3. Update the flashcards with key terms and concepts'); | ||
| steps.push('4. Search and add one relevant YouTube video if possible'); | ||
| } |
There was a problem hiding this comment.
Invalid genTypes not detected
GenerateStudyMaterialsHandler treats any non-empty genTypes entries as recognized and builds steps off types.has(...). For ?genTypes=foo,bar (or whitespace variants), types is non-null and steps becomes empty, which triggers the fallback-to-all path, but CreateFromPromptHandler does not have an equivalent fallback: it will omit the YouTube suffix because types is non-null and includes('youtube') is false. That makes the first user message not match CREATE_FROM_REGEX (server still expects a trailing period) and getCreateFromSystemPrompt won’t inject the initialization instructions. Net effect: createFrom auto-init can silently lose the server-side system prompt when genTypes contains only invalid values.
1 participant
Important
Enhances auto-generation of personalized content in workspaces with user-selectable content types and improved handling of URL parameters.
HomePromptInput.tsxfor users to select content types (notes, quizzes, flashcards, YouTube) with persistence.route.tsandAssistantPanel.tsx, reflecting selected types in prompts and creation flows.WorkspaceRuntimeProvider.tsxandAssistantPanel.tsx, ensuring scoped output.AssistantPanel.tsx.CREATE_FROM_REGEXinroute.tsto make YouTube suffix optional.Popoverfor generation settings inHomePromptInput.tsx.genTypesfrom URL after use inAssistantPanel.tsxandWorkspaceRuntimeProvider.tsx.This description was created by
for f1cc7e5. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes