Improvements

[Jeadie]

This PR build off of #60, it will be easier to read once it's merged.

Improvements

• Basic, extendable support to use any LLM model you like
• Better typing, structs, enums etc. to make the code more understandable.
• Better package layout

[patillacode]

Hi @Jeadie

Thanks for your contribution, as you can see I just merged #60

In order to merge this PR with the improvements please:

• rebase from main (all extra commits should disappear)
• fix conflicts

Also, I left a comment!

[patillacode]

LGTM

[patillacode]

Thanks @Jeadie

@AntonOsika I would like your confirmation on this one before I merge.

[AntonOsika]

Thanks for the good work keeping this up with main @Jeadie – and wanting to get it merged!

I added feedback on how we can get this in a really good update.

What I also propose:

• We first merge the changes to the types
• Then we look at the PR that introduce some complexity so that we can have multiple types of LLMs! (which will be very useful in the future)

Would you be able to look through my comments, see if you agree with the proposed dataclassses, and if so create a new PR that only updates the types?

[AntonOsika]

I realised we can just use json.dumps(dataclasses.asdict()), no need for additional dependencies 👌

[Jeadie]

I had the growing suspicion as I worked on this that Tuple[Role, Message] should become a complete dataclass. I started this change to make the function signatures more understandable and typed, and to be more robust, when/if a Role or Message get more complex.

[Jeadie]

Suggested change

	class Step(Enum):
	CLARIFY = "clarify"
	RUN_CLARIFIED = "run_clarified"

[Jeadie]

I think I will move the support for mulitple models to a new PR, just focus this one on improving typing etc.

[patillacode]

@Jeadie

I took the liberty of fixing a couple of simple conflicts (requirements related), please make sure to pull!

Also, it seems mypy identified an issue

[AntonOsika]

Ping me when I should review!

[AntonOsika]

Curious about progress @Jeadie

[AntonOsika]

My idea of introducing dataclass of Messages needs quite some work with appending messages so I'd be happy to reconsider

[AntonOsika]

Hey! Good ideas here. I'm not sure if we should merge or if we should use the ideas and do a new PR altogether. For example, maybe we want to put messages in its own file and give it classmethods like "to_dict" etc.

Regardless: before merging, would be good to look at how e.g. langchain and others handles this.

[Jeadie]

I agree

[Jeadie]

Looking into Langchain, this is their data structures for their messages, source.

class BaseMessage(Serializable):
    """Message object."""

    content: str
    additional_kwargs: dict = Field(default_factory=dict)

    @property
    @abstractmethod
    def type(self) -> str:
        """Type of the message, used for serialization."""

    @property
    def lc_serializable(self) -> bool:
        """This class is LangChain serializable."""
        return True

Subclasses are for various use cases, for example: HumanMessage, AIMessage, FunctionMessage. These messages are based off a Serializable class that inherits from pydantic and an abstract class source.

It appears to be a similar approach. One difference is the use of a type property instead of a separate type/role enum. For example, in LangChain

@property
def type(self) -> str:
    """Type of the message, used for serialization."""
    return "human"

[Jeadie]

These then get used internally per specific use cases, which provide public methods and uses these message types internally. For example (source)

class BaseChatMessageHistory(ABC):
    """Base interface for chat message history
    See `ChatMessageHistory` for default implementation.
    """

    """
    Example:
        .. code-block:: python

            class FileChatMessageHistory(BaseChatMessageHistory):
                storage_path:  str
                session_id: str

               @property
               def messages(self):
                   with open(os.path.join(storage_path, session_id), 'r:utf-8') as f:
                       messages = json.loads(f.read())
                    return messages_from_dict(messages)

               def add_message(self, message: BaseMessage) -> None:
                   messages = self.messages.append(_message_to_dict(message))
                   with open(os.path.join(storage_path, session_id), 'w') as f:
                       json.dump(f, messages)
               
               def clear(self):
                   with open(os.path.join(storage_path, session_id), 'w') as f:
                       f.write("[]")
    """

    messages: List[BaseMessage]

    def add_user_message(self, message: str) -> None:
        """Add a user message to the store"""
        self.add_message(HumanMessage(content=message))

     ...

[AntonOsika]

Makes sense.

We really value simplicity so I am inclined to u turn in the name of simplicity here and just have a Message class and use lists like you suggested initially.

[AntonOsika]

I.e., just merging this part first!^

Would be an improvement for sure

[AntonOsika]

Thanks for the contributions @Jeadie helped us realised what we want!

Moving discussion to #466

Will close since it's merge, feel free to reopen.