Make some improvements to Corpora Commander, playing with agents

[skyl]

PR Type

Enhancement, Documentation

---

Description

• Enhance image management with new features

• Add detailed image token editing capabilities

• Introduce Copilot instructions for setup and development

• Improve UI layout and interactions

---

Diagram Walkthrough

flowchart LR
  A["ImageDrawer.tsx"] -- "Refactor and enhance" --> B["ImageTokenList.tsx"]
  B -- "Add new features" --> C["ImageDetailModal"]
  D["Copilot Instructions"] -- "Add documentation" --> E[".github/copilot-instructions.md"]

Loading

File Walkthrough

	Relevant files
Enhancement	ImageDrawer.tsx Refactor and enhance ImageDrawer component                              ts/commander/src/components/ImageDrawer.tsx Refactor to use useGenerateImage hook Add handling for generating missing tokens Simplify UI layout and remove unused components +33/-35  ImageTokenList.tsx Add detailed image token editing and management                    ts/commander/src/components/ImageTokenList.tsx Add detailed image token editing capabilities Implement token deletion and editing Enhance UI with drag-and-drop support +565/-70
Documentation	copilot-instructions.md Add Copilot setup and development instructions                      .github/copilot-instructions.md Add setup instructions for Copilot Include environment and bootstrap steps Document dev server commands and formatting +36/-0

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 81aa1ef)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Issue The handleGenerateMissing function does not handle errors that might occur during the generate.mutate call. Consider adding error handling to improve robustness. const handleGenerateMissing = () => { if (missingTokens.length === 0) return; missingTokens.forEach((t) => generate.mutate(t.caption, promptValue)); Performance Concern The handleDeleteToken and handleSaveEdit functions perform multiple asynchronous operations in a loop without any concurrency control, which could lead to performance issues. Consider using Promise.all or similar to handle these operations concurrently. const handleDeleteToken = async (caption: string, occurrences?: ImageTokenOccurrence[]) => { if (!occurrences || occurrences.length === 0) return; const ok = window.confirm( `Remove all {{IMAGE: ${caption}}} tokens from their sections/subsections?`, ); if (!ok) return; const targets = new Map< string, { sectionId: string; subsectionId?: string | null } >(); occurrences.forEach((occ) => { const key = occ.subsection_id ? `sub:${occ.subsection_id}` : `sec:${occ.section_id}`; if (!targets.has(key)) { targets.set(key, { sectionId: occ.section_id, subsectionId: occ.subsection_id ?? null, }); } }); setDeletingToken(caption); try { for (const target of targets.values()) { if (target.subsectionId) { const subRes = await corporaCommanderApiSubsectionGetSubsection( target.subsectionId, ); const sub = subRes.data; const content = removeImageToken(sub.content ?? "", caption); const instructions = removeImageToken( sub.instructions ?? "", caption, ); if (!content.changed && !instructions.changed) continue; const payload: { content?: string; instructions?: string; } = {}; if (content.changed) payload.content = content.text; if (instructions.changed) payload.instructions = instructions.text; await updateSubsection.mutateAsync({ subsectionId: target.subsectionId, data: payload, }); } else { const secRes = await corporaCommanderApiSectionGetSection( target.sectionId, ); const sec = secRes.data; const intro = removeImageToken(sec.introduction ?? "", caption); const instructions = removeImageToken( sec.instructions ?? "", caption, ); if (!intro.changed && !instructions.changed) continue; const payload: { introduction?: string; instructions?: string; } = {}; if (intro.changed) payload.introduction = intro.text; if (instructions.changed) payload.instructions = instructions.text; await updateSection.mutateAsync({ sectionId: target.sectionId, data: payload, }); } } } finally { setDeletingToken(null); queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiImagesListImageTokensQueryKey( projectId, ), }); } }; const handleEditToken = (token: ImageToken) => { setEditingToken(token.caption); setEditingValue(token.caption); }; const handleCancelEdit = () => { setEditingToken(null); setEditingValue(""); }; const handleSaveEdit = async (token: ImageToken) => { const nextCaption = editingValue.trim(); if (!nextCaption || nextCaption === token.caption) { handleCancelEdit(); return; } setSavingToken(token.caption); try { const occurrences = token.occurrences ?? []; const targets = new Map< string, { sectionId: string; subsectionId?: string | null } >(); occurrences.forEach((occ) => { const key = occ.subsection_id ? `sub:${occ.subsection_id}` : `sec:${occ.section_id}`; if (!targets.has(key)) { targets.set(key, { sectionId: occ.section_id, subsectionId: occ.subsection_id ?? null, }); } }); for (const target of targets.values()) { if (target.subsectionId) { const subRes = await corporaCommanderApiSubsectionGetSubsection( target.subsectionId, ); const sub = subRes.data; const content = replaceImageToken( sub.content ?? "", token.caption, nextCaption, ); const instructions = replaceImageToken( sub.instructions ?? "", token.caption, nextCaption, ); if (!content.changed && !instructions.changed) continue; const payload: { content?: string; instructions?: string; } = {}; if (content.changed) payload.content = content.text; if (instructions.changed) payload.instructions = instructions.text; await updateSubsection.mutateAsync({ subsectionId: target.subsectionId, data: payload, }); queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiSubsectionGetSubsectionQueryKey( target.subsectionId, ), }); } else { const secRes = await corporaCommanderApiSectionGetSection( target.sectionId, ); const sec = secRes.data; const intro = replaceImageToken( sec.introduction ?? "", token.caption, nextCaption, ); const instructions = replaceImageToken( sec.instructions ?? "", token.caption, nextCaption, ); if (!intro.changed && !instructions.changed) continue; const payload: { introduction?: string; instructions?: string; } = {}; if (intro.changed) payload.introduction = intro.text; if (instructions.changed) payload.instructions = instructions.text; await updateSection.mutateAsync({ sectionId: target.sectionId, data: payload, }); queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiSectionGetSectionQueryKey( target.sectionId, ), }); } } if (token.image_id) { await updateImage.mutateAsync({ projectId, imageId: token.image_id, data: { caption: nextCaption }, }); } queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiImagesListImagesQueryKey(projectId), }); queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiImagesListImageTokensQueryKey( projectId, ), }); queryClient.invalidateQueries({ queryKey: getCorporaCommanderApiSectionListSectionsQueryKey(projectId), }); } finally { setSavingToken(null); setEditingToken(null); setEditingValue(""); }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 81aa1ef
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Check promptValue before mutation Ensure that generate.mutate is called only if promptValue is not undefined to prevent potential errors during image generation. ts/commander/src/components/ImageDrawer.tsx [50-51] -missingTokens.forEach((t) => generate.mutate(t.caption, promptValue)); +if (promptValue) { + missingTokens.forEach((t) => generate.mutate(t.caption, promptValue)); +} Suggestion importance[1-10]: 7 __ Why: The suggestion to check if promptValue is not undefined before calling generate.mutate is valid and helps prevent potential runtime errors during image generation, improving code robustness.	Medium
	Validate promptValue before mutation Add a check to ensure promptValue is not undefined before calling generate.mutate to avoid errors in the image generation process. ts/commander/src/components/ImageTokenList.tsx [69-71] const handleGenerate = (caption: string) => { - generate.mutate(caption, promptValue); + if (promptValue) { + generate.mutate(caption, promptValue); + } }; Suggestion importance[1-10]: 7 __ Why: Adding a check for promptValue before calling generate.mutate ensures that the function does not encounter undefined values, which could lead to errors, thus enhancing the reliability of the image generation process.	Medium

---

Previous suggestions

Suggestions up to commit 2f39da2

Category	Suggestion	Impact
General	Replace window.confirm with custom dialog Avoid using window.confirm for user confirmations as it can be disruptive and is not user-friendly. Consider using a custom modal dialog for confirmations to provide a more consistent user experience. ts/commander/src/components/ImageTokenList.tsx [108-110] -const ok = window.confirm( - `Remove all {{IMAGE: ${caption}}} tokens from their sections/subsections?`, +const ok = await showConfirmationDialog( + `Remove all {{IMAGE: ${caption}}} tokens from their sections/subsections?` ); Suggestion importance[1-10]: 7 __ Why: Replacing window.confirm with a custom dialog can enhance user experience by providing a more consistent and less disruptive interface. This change is beneficial for usability but does not address a critical issue, hence a moderate score. The suggestion is accurate and contextually appropriate.	Medium

[github-actions]

Persistent review updated to latest commit 81aa1ef

[skyl]

• persist image instructions on project - localstorage is enough I think - in the images manager.
• confirmation of delete (main section trashcan for section)
• different size books (render header template based on project config)
• name history snapshots