Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for an OpenAI-compatible thinking mode, adds a fallback literal PDF text extractor, implements an IndexedDB-based storage fallback for large slide images, and updates the web UI with a new warm theme and an image lightbox component. Key feedback includes: only passing the thinking parameter in extra_body when explicitly enabled to prevent breaking standard OpenAI endpoints; avoiding bypassing onSelect in SlideCard to keep slide navigation functional; optimizing IndexedDB operations in StorageService by reusing a single connection and clearing the store instead of deleting the database; and removing the fragile fallback PDF extractor in favor of returning None when pypdf is unavailable.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if self.config.text_thinking in {"enabled", "disabled"}: | ||
| kwargs["extra_body"] = {"thinking": {"type": self.config.text_thinking}} |
There was a problem hiding this comment.
Passing extra_body with "thinking": {"type": "disabled"} by default when thinking is disabled will break compatibility with standard OpenAI-compatible endpoints that do not support the thinking parameter (resulting in a 400 Bad Request). We should only pass the thinking parameter in extra_body when it is explicitly set to "enabled".
| if self.config.text_thinking in {"enabled", "disabled"}: | |
| kwargs["extra_body"] = {"thinking": {"type": self.config.text_thinking}} | |
| if self.config.text_thinking == "enabled": | |
| kwargs["extra_body"] = {"thinking": {"type": "enabled"}} |
| if profile.thinking in {"enabled", "disabled"}: | ||
| kwargs["extra_body"] = {"thinking": {"type": profile.thinking}} |
There was a problem hiding this comment.
Similar to the client configuration, passing extra_body with "thinking": {"type": "disabled"} by default will break compatibility with standard OpenAI-compatible endpoints. We should only pass it when thinking is explicitly set to "enabled".
| if profile.thinking in {"enabled", "disabled"}: | |
| kwargs["extra_body"] = {"thinking": {"type": profile.thinking}} | |
| if profile.thinking == "enabled": | |
| kwargs["extra_body"] = {"thinking": {"type": "enabled"}} |
| const handleClick = () => { | ||
| onSelect(slide.id) | ||
| if (canEdit && onEdit) { | ||
| onEdit(slide.id) | ||
| } else { | ||
| onSelect(slide.id) | ||
| } | ||
| } |
There was a problem hiding this comment.
Bypassing onSelect when onEdit is provided breaks slide selection/navigation in the right panel gallery. It also makes the hover edit button redundant since clicking anywhere on the card would trigger the edit panel. Reverting this to only trigger onSelect ensures that selection works correctly and editing is initiated via the dedicated edit button.
const handleClick = () => {
onSelect(slide.id)
}
| static async loadProjectWithImages(): Promise<PersistedState['currentProject']> { | ||
| const project = StorageService.loadProject() | ||
| if (!project) { | ||
| return null | ||
| } | ||
|
|
||
| if (StorageService.pendingImageSave) { | ||
| await StorageService.pendingImageSave | ||
| } | ||
|
|
||
| const slides = await Promise.all(project.slides.map(async (slide) => { | ||
| if (slide.imageBase64 || slide.imageUrl) { | ||
| return slide | ||
| } | ||
| if (!slide.imageStorageKey) { | ||
| return slide | ||
| } | ||
| const imageBase64 = await StorageService.loadSlideImage(slide.imageStorageKey) | ||
| if (!imageBase64) { | ||
| return slide | ||
| } | ||
| return { | ||
| ...slide, | ||
| imageBase64, | ||
| imageUrl: `data:image/png;base64,${imageBase64}` | ||
| } | ||
| })) | ||
|
|
||
| return { | ||
| ...project, | ||
| slides | ||
| } | ||
| } |
There was a problem hiding this comment.
Calling StorageService.loadSlideImage inside Promise.all opens and closes a separate IndexedDB connection for every single slide image concurrently. This is highly inefficient and can cause performance bottlenecks or connection exhaustion. We should open the database connection once in loadProjectWithImages, perform all reads, and then close it.
static async loadProjectWithImages(): Promise<PersistedState['currentProject']> {
const project = StorageService.loadProject()
if (!project) {
return null
}
if (StorageService.pendingImageSave) {
await StorageService.pendingImageSave
}
let db: IDBDatabase | null = null
try {
if (StorageService.canUseIndexedDb()) {
db = await StorageService.openImageDb()
}
} catch (e) {
console.error('Failed to open image DB:', e)
}
const slides = await Promise.all(project.slides.map(async (slide) => {
if (slide.imageBase64 || slide.imageUrl) {
return slide
}
if (!slide.imageStorageKey || !db) {
return slide
}
const imageBase64 = await new Promise<string | null>((resolve) => {
try {
const transaction = db!.transaction(IMAGE_DB_STORE, 'readonly')
const store = transaction.objectStore(IMAGE_DB_STORE)
const request = store.get(slide.imageStorageKey!)
request.onsuccess = () => resolve((request.result as SlideImageRecord | undefined)?.imageBase64 || null)
request.onerror = () => resolve(null)
} catch {
resolve(null)
}
})
if (!imageBase64) {
return slide
}
return {
...slide,
imageBase64,
imageUrl: `data:image/png;base64,${imageBase64}`
}
}))
if (db) {
db.close()
}
return {
...project,
slides
}
}| private static clearImageStore(): void { | ||
| if (!StorageService.canUseIndexedDb()) { | ||
| return | ||
| } | ||
| const request = indexedDB.deleteDatabase(IMAGE_DB_NAME) | ||
| request.onerror = () => { | ||
| console.error('Failed to clear slide image store:', request.error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Deleting the entire database with indexedDB.deleteDatabase can be blocked if there are other open connections (e.g., in other tabs or pending operations in the current tab). It is much safer and more robust to open the database and clear the object store using store.clear().
private static clearImageStore(): void {
if (!StorageService.canUseIndexedDb()) {
return
}
StorageService.openImageDb().then((db) => {
const transaction = db.transaction(IMAGE_DB_STORE, 'readwrite')
const store = transaction.objectStore(IMAGE_DB_STORE)
store.clear()
transaction.oncomplete = () => db.close()
transaction.onerror = () => db.close()
}).catch((error) => {
console.error('Failed to clear slide image store:', error)
})
}| try: | ||
| from pypdf import PdfReader | ||
| except ImportError: | ||
| return None | ||
| return self._parse_literal_pdf_text(path) |
There was a problem hiding this comment.
The fallback literal PDF text extractor is extremely fragile. It does not handle FlateDecode stream compression (which is standard for almost all modern PDFs), hexadecimal strings, TJ operators, or escaped parentheses correctly. If pypdf is not available, it is much safer to return None rather than attempting a naive regex-based binary parse that will almost always fail or return garbage.
| try: | |
| from pypdf import PdfReader | |
| except ImportError: | |
| return None | |
| return self._parse_literal_pdf_text(path) | |
| try: | |
| from pypdf import PdfReader | |
| except ImportError: | |
| return None |
There was a problem hiding this comment.
Pull request overview
This PR improves persistence and restoration of generated slide images by offloading oversized base64 payloads to IndexedDB when localStorage quota is exceeded, then rehydrating images during session restore so decks don’t remain stuck on placeholders. It also updates the UI/theme styling and introduces an OpenAI-compatible “thinking” mode flag that flows through both frontend model-profile requests and backend routing.
Changes:
- Add IndexedDB-backed storage for slide image base64 data during quota fallback, plus restoration via
loadProjectWithImages(). - Introduce/propagate
thinking: enabled|disabledacross web config, model profiles, and backend OpenAI calls. - Refresh UI styling and add an image lightbox for viewing/downloading slide images.
Reviewed changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/utils/apiConfig.ts | Updates default/full API config shape and migrates legacy thinkingLevel into new thinking mode. |
| web/src/types/index.ts | Adds imageStorageKey on slides and replaces thinkingLevel with thinking in relevant config types. |
| web/src/services/storageService.ts | Implements IndexedDB slide-image storage + project load that rehydrates images from IndexedDB. |
| web/src/services/modelProfileService.ts | Sends thinking in prompt model profile payload to backend. |
| web/src/services/generateService.ts | Sends thinking in generation config payload; formatting/typing cleanups. |
| web/src/index.css | Major theme/style variable updates and new utility classes for cards/panels. |
| web/src/i18n.ts | Adds/updates strings for theme mode, thinking mode, missing images, lightbox actions, and UI labels. |
| web/src/hooks/useStateRestore.ts | Restores projects via loadProjectWithImages() and adds cancellation guard. |
| web/src/components/SlideList.tsx | Styling tweaks for slide list layout. |
| web/src/components/SlideCard.tsx | Adds missing-image state, preview lightbox entry point, and click behavior changes. |
| web/src/components/RightPanel.tsx | Shifts to gallery-first presentation and updates empty/loading states. |
| web/src/components/LeftPanel.tsx | Styling/layout adjustments. |
| web/src/components/Layout.tsx | Replaces theme dropdown with a theme toggle switch and updates layout spacing. |
| web/src/components/ImageLightbox.tsx | New component to preview and download slide images. |
| web/src/components/GenerationConfigForm.tsx | Styling updates to match new theme primitives. |
| web/src/components/GenerateButton.tsx | Styling updates for disabled/active states. |
| web/src/components/FileUpload.tsx | Styling updates for dropzone and file info display. |
| web/src/components/FilePreview.tsx | Styling updates for markdown-ish preview rendering. |
| web/src/components/ExportButton.tsx | Styling updates for button + dropdown menu. |
| web/src/components/EditPanel.tsx | Adds lightbox preview for original/edited images; updates layout and copy. |
| web/src/components/EditHistory.tsx | Styling updates and improved layout for history items. |
| web/src/components/CenterPanel.tsx | Styling/layout updates. |
| web/src/components/ApiConfigForm.tsx | Updates UI to configure thinking mode and adjusts styling throughout. |
| web/src/components/tests/StatePersistence.property.test.tsx | Extends persistence property test to cover IndexedDB image restoration. |
| web/package.json | Adds fake-indexeddb dev dependency for tests. |
| web/package-lock.json | Locks fake-indexeddb dependency resolution. |
| tests/test_model_router.py | Adds backend unit test asserting thinking is passed via extra_body. |
| tests/test_config_writer.py | Updates config writer tests to persist thinking for prompt model. |
| src/model_router.py | Passes thinking into OpenAI calls via extra_body when configured. |
| src/model_profiles.py | Adds thinking field to model profiles with validation/defaulting. |
| src/document_parser.py | Adds regex-based literal PDF text extraction as a fallback when pypdf fails/unavailable. |
| src/config.py | Adds text_thinking alongside legacy text_thinking_level config. |
| src/config_writer.py | Persists thinking into config.yaml for prompt model profiles. |
| src/client.py | Passes text_thinking into OpenAI calls via extra_body. |
| config.example.yaml | Documents new thinking setting in example config. |
| api/profile_resolver.py | Propagates thinking from request config into model profiles. |
| api/models.py | Adds thinking to request schema and a get_thinking() helper. |
Files not reviewed (1)
- web/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
web/src/services/storageService.ts:260
- When the initial
writeState(newState, false)fails (quota), the code kicks offsaveSlideImages()but does not wait for it before persisting the compact state withimageStorageKeys. If the user reloads/closes the tab immediately aftersaveProject()returns, localStorage may reference keys that were never committed to IndexedDB yet, resulting in missing images despite a “successful” save. Consider making the compaction path await the IndexedDB transaction (or otherwise guaranteeing durability before returning success).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleClick = () => { | ||
| onSelect(slide.id) | ||
| if (canEdit && onEdit) { | ||
| onEdit(slide.id) | ||
| } else { | ||
| onSelect(slide.id) | ||
| } |
| private static imageKey(fileName: string, slide: Slide): string { | ||
| const projectName = fileName || 'untitled' | ||
| return `${projectName}:${slide.id}` | ||
| } |
| def _parse_literal_pdf_text(self, path: Path) -> Optional[ParsedDocument]: | ||
| fallback_text = self._extract_literal_pdf_text(path) | ||
| if not fallback_text: | ||
| return None | ||
| return ParsedDocument( | ||
| filename=path.name, | ||
| normalized_markdown=f"<!-- page: 1 -->\n{fallback_text}", | ||
| metadata={"parser": "pypdf", "extension": ".pdf", "pages": 1}, | ||
| ) |
Summary
Test Plan