feat: implement unified chat context design #2820
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add context refs state to useChatMode (refs, addRef, removeRef, clearRefs) - Create ContextBar component to display context refs as chips - Implement auto-sync from open tabs to refs - Move resolution logic to useTransport (lazy loading at send time) - Update slash command to add refs instead of inserting text for sessions - Remove old attachedSessionId flow from ChatMessageInput Co-Authored-By: yujonglee <[email protected]>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for howto-fix-macos-audio-selection canceled.
|
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
- Use main.UI.useIndexes to get the indexes instance - Call getSliceRowIds on indexes instead of store - Remove unused useMemo import from view.tsx Co-Authored-By: yujonglee <[email protected]>
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+58
to
+69
| const addRef = useCallback((ref: ContextRef) => { | ||
| setRefs((prev) => { | ||
| if (prev.some((r) => r.type === ref.type && r.id === ref.id)) { | ||
| return prev; | ||
| } | ||
| return [...prev, ref]; | ||
| }); | ||
| }, []); | ||
|
|
||
| const removeRef = useCallback((id: string) => { | ||
| setRefs((prev) => prev.filter((r) => r.id !== id)); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Duplicate detection vs removal logic mismatch
addRef prevents duplicates by checking both type and id (line 60), but removeRef only filters by id (line 68). This creates an inconsistency:
- If two refs exist with the same
idbut differenttype(e.g.,{type: "session", id: "123"}and{type: "human", id: "123"}), they can coexist - But calling
removeRef("123")will remove BOTH refs, not just the intended one - In
context-bar.tsxline 22, clicking remove on one chip will unexpectedly remove other chips with the same ID
// Fix: Match the removal logic to the duplicate check
const removeRef = useCallback((type: ContextRefType, id: string) => {
setRefs((prev) => prev.filter((r) => !(r.type === type && r.id === id)));
}, []);
Suggested change
| const addRef = useCallback((ref: ContextRef) => { | |
| setRefs((prev) => { | |
| if (prev.some((r) => r.type === ref.type && r.id === ref.id)) { | |
| return prev; | |
| } | |
| return [...prev, ref]; | |
| }); | |
| }, []); | |
| const removeRef = useCallback((id: string) => { | |
| setRefs((prev) => prev.filter((r) => r.id !== id)); | |
| }, []); | |
| const addRef = useCallback((ref: ContextRef) => { | |
| setRefs((prev) => { | |
| if (prev.some((r) => r.type === ref.type && r.id === ref.id)) { | |
| return prev; | |
| } | |
| return [...prev, ref]; | |
| }); | |
| }, []); | |
| const removeRef = useCallback((type: ContextRefType, id: string) => { | |
| setRefs((prev) => prev.filter((r) => !(r.type === type && r.id === id))); | |
| }, []); |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the chat context system to store references (IDs) instead of content, resolving content only at send time. This addresses issues with stale context, hidden context visibility, and lack of manual control over what context is attached to chat.
Key changes:
ContextReftype and state management inuseChatMode(refs, addRef, removeRef, clearRefs)useTransportnow accepts refs and resolves content from store/indexes at send time/now adds it as a context ref instead of inserting a mention (newonSelectcallback in tiptap)Review & Testing Checklist for Human
/and select a session - verify it adds to ContextBar (not inserted as text). This uses a newonSelectcallback in the tiptap mention extension.Notes
The resolution logic in
useTransportwas changed from using React hooks (useSession,main.UI.useSliceRowIds) to direct store/indexes access (store.getRow,indexes.getSliceRowIds). This is intentional to enable lazy resolution but changes the reactivity model.Updates since last revision:
main.UI.useIndexes()to get the indexes instance for callinggetSliceRowIds, rather than incorrectly calling it on the store object.Link to Devin run: https://app.devin.ai/sessions/e583e4169beb43088b5d3b58fcb95ec2
Requested by: @yujonglee ([email protected])