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

Subissue: Lift chat input state to ChatModel #163

Closed
dlqqq opened this issue Feb 11, 2025 · 2 comments · Fixed by #171
Closed

Subissue: Lift chat input state to ChatModel #163

dlqqq opened this issue Feb 11, 2025 · 2 comments · Fixed by #171
Assignees
Labels

Comments

@dlqqq
Copy link
Member

dlqqq commented Feb 11, 2025

Problem

While building #161, I noticed that I had to pass input and setInput from the <ChatInput /> component to the chat command providers, since they need to read & write from the input. This results in a weird interface:

export interface IChatCommandProvider {
    id: string;
    ready?: Promise<void>;
    getChatCommands(partialInput: string): Promise<ChatCommand[]>;
    handleChatCommand(command: ChatCommand, partialInput: string, replacePartialInput: (newPartialInput: string) => void): Promise<void>;
}

If a ChatModel instance could provide methods for reading & writing to the input, the interface can then be simplified to:

export interface IChatCommandProvider {
    id: string;
    ready?: Promise<void>;
    getChatCommands(chatModel: IChatModel): Promise<ChatCommand[]>;
    handleChatCommand(command: ChatCommand, chatModel: IChatModel): Promise<void>;
}

Proposed Solution

Lift chat input state up to the ChatModel instance. The <ChatInput /> component should be updated accordingly.

Additional context

Chat commands PR: #161

@dlqqq dlqqq added the enhancement New feature or request label Feb 11, 2025
@dlqqq dlqqq changed the title Lift chat input state to ChatModel Subissue: Lift chat input state to ChatModel Feb 14, 2025
@dlqqq
Copy link
Member Author

dlqqq commented Feb 14, 2025

Discussed this briefly with @brichet on Thursday 02/13. Nicolas mentioned that it may make more sense for this to live in a InputModel under the ChatModel. This sounds reasonable to me since it namespaces methods for getting & setting the input under chatModel.input.

@dlqqq
Copy link
Member Author

dlqqq commented Feb 14, 2025

@brichet Here is an interface that describes the methods & properties needed by the Chat Commands framework:

interface IInputModel {
  /* the entire input */
  get value(): string
  set value(newInput: string): void
  valueChanged: ISignal<IInputModel, string>;

  /**
   * the current cursor index.
   * this refers to the index of the character in front of the cursor.
   */
  get cursorIndex(): number
  set cursorIndex(newIndex: number): void
  cursorIndexChanged: ISignal<IInputModel, number>

  /**
   * the current word behind the user's cursor, space-separated.
   * should call functions defined under `input/utils.ts`.
   */
  get currentWord(): string
  set currentWord(newWord: string): void
  currentWordChanged: ISignal<IInputModel, string>;
} 

@dlqqq dlqqq moved this to Todo in Jupyter AI Feb 17, 2025
@dlqqq dlqqq added this to Jupyter AI Feb 17, 2025
@dlqqq dlqqq moved this from Todo to In Progress in Jupyter AI Feb 17, 2025
@dlqqq dlqqq moved this from In Progress to Needs Review in Jupyter AI Feb 18, 2025
@dlqqq dlqqq moved this from Needs Review to In Review in Jupyter AI Feb 18, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in Jupyter AI Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants