-
Notifications
You must be signed in to change notification settings - Fork 45
fix(sessions): name tasks from pasted text-file contents #2850
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import { create } from "zustand"; | ||
|
|
||
| /** | ||
| * Local attachment file paths captured at task-creation time, keyed by task id, | ||
| * so the chat-title generator can read their contents when naming a task. | ||
| * | ||
| * Why this exists: when a prompt is pasted as a text file (or otherwise sent as | ||
| * an attachment with no typed text), the title has to come from the file's | ||
| * contents. For local tasks the prompt event still carries the `<file .../>` | ||
| * path, so the generator can read it directly. For cloud tasks it cannot — the | ||
| * stored description is reduced to `Attached files: <name>` and the echoed | ||
| * prompt event points at the remote sandbox path (e.g. | ||
| * `file:///workspace/.posthog/attachments/...`), which is not readable on the | ||
| * user's machine. The only place the original local path exists is at submit | ||
| * time, so we stash it here and hand it to the generator, which reads the file | ||
| * locally before it is cleaned up. | ||
| * | ||
| * Best-effort and in-memory: lost on reload, at which point the title falls back | ||
| * to the attachment summary. | ||
| */ | ||
| interface TitleAttachmentStore { | ||
| byTaskId: Record<string, string[]>; | ||
| set: (taskId: string, filePaths: string[]) => void; | ||
| get: (taskId: string) => string[] | undefined; | ||
| clear: (taskId: string) => void; | ||
| } | ||
|
|
||
| export const useTitleAttachmentStore = create<TitleAttachmentStore>( | ||
| (set, get) => ({ | ||
| byTaskId: {}, | ||
| set: (taskId, filePaths) => | ||
| set((state) => ({ | ||
| byTaskId: { ...state.byTaskId, [taskId]: filePaths }, | ||
| })), | ||
| get: (taskId) => get().byTaskId[taskId], | ||
| clear: (taskId) => | ||
| set((state) => { | ||
| if (!(taskId in state.byTaskId)) return state; | ||
| const { [taskId]: _removed, ...rest } = state.byTaskId; | ||
| return { byTaskId: rest }; | ||
| }), | ||
| }), | ||
| ); | ||
|
Comment on lines
+28
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
|
|
||
| export const titleAttachmentStoreApi = { | ||
| set: (taskId: string, filePaths: string[]) => | ||
| useTitleAttachmentStore.getState().set(taskId, filePaths), | ||
| get: (taskId: string) => useTitleAttachmentStore.getState().get(taskId), | ||
| clear: (taskId: string) => useTitleAttachmentStore.getState().clear(taskId), | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
titleAttachmentStoreApi.clear(taskId)is only called insideif (result), so whengenerateTitleAndSummaryreturnsnull(on error or empty output), the stash entry survives. In theshouldGenerateFromTaskDescriptionpath,initialDescriptionHandledis unconditionally set totruein thefinallyblock, which prevents any description-path retry. The stash therefore stays populated for the rest of the session without any mechanism to consume or evict it other than the next prompt-based generation or a reload. This is acknowledged as best-effort, but moving theclearcall to thefinallyblock (only for the description path, not if a retry on prompts is still wanted) would make the lifecycle explicit.