Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Writing assistant v2 #487

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft

Writing assistant v2 #487

wants to merge 22 commits into from

Conversation

samlhuillier
Copy link
Collaborator

No description provided.

@samlhuillier samlhuillier marked this pull request as draft November 13, 2024 23:04
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added AI-powered text editing capabilities and document statistics while removing the backlink suggestions system.

  • New AIEdit.tsx component sends raw text to /api/generate endpoint without content validation or sanitization, potential security concern
  • Missing error handling and loading states in AI editing operations in AIEdit.tsx
  • Event listener cleanup missing in DocumentStats.tsx useEffect hook, potential memory leak
  • Empty onEdit callback in EditorManager.tsx for AI edit menu makes feature non-functional
  • Continue writing button in AI menu lacks implementation

4 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 17 to 23
const handleAction = async (action: string) => {
const prompt = `${action} the following text: ${selectedText}`
const completion = await complete(prompt)
if (completion) {
onEdit(completion)
}
}
Copy link

Choose a reason for hiding this comment

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

logic: no error handling for failed API calls or rate limits. Add try/catch and show error state to user


const handleAction = async (action: string) => {
const prompt = `${action} the following text: ${selectedText}`
const completion = await complete(prompt)
Copy link

Choose a reason for hiding this comment

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

logic: missing loading state while waiting for completion. User has no feedback during API call

Comment on lines 75 to 78
<Button variant="ghost" className="w-full justify-start text-gray-300 hover:bg-gray-800">
<Play className="mr-2 size-5 text-purple-500" />
Continue writing
</Button>
Copy link

Choose a reason for hiding this comment

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

logic: 'Continue writing' button has no onClick handler - appears to be non-functional

Comment on lines +11 to +24
useEffect(() => {
const initDocumentStats = async () => {
const showStats = await window.electronStore.getDocumentStats()
setShow(showStats)
}

initDocumentStats()

const handleDocStatsChange = (event: Electron.IpcRendererEvent, value: boolean) => {
setShow(value)
}

window.ipcRenderer.on('show-doc-stats-changed', handleDocStatsChange)
}, [])
Copy link

Choose a reason for hiding this comment

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

logic: missing cleanup function to remove IPC event listener on unmount

}}
>
{showAIPopup ? (
<AiEditMenu selectedText={editor.getText()} onEdit={() => {}} />
Copy link

Choose a reason for hiding this comment

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

logic: onEdit prop is passed an empty function - AI edits will not be applied to the editor

className="flex gap-2 rounded-lg border border-gray-700 bg-dark-gray-c-eleven p-2 shadow-lg"
editor={editor}
tippyOptions={{
duration: 1000,
Copy link

Choose a reason for hiding this comment

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

style: 1000ms duration for bubble menu animation may feel slow for frequent interactions

@@ -52,26 +42,40 @@ const EditorManager: React.FC = () => {
window.ipcRenderer.on('editor-flex-center-changed', handleEditorChange)
}, [])
Copy link

Choose a reason for hiding this comment

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

logic: missing cleanup for ipcRenderer event listener

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant