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

move process_message inside BaseLLM #1021

Closed
wants to merge 5 commits into from

Conversation

geohotstan
Copy link

@geohotstan geohotstan commented Mar 17, 2024

Features

Feature Docs

Influence

Result

  • This PR fixes this specific error KeyError: "Could not recognize the intended type of the dict. A Content should have a 'parts' key. A Part should have a 'inline_data' or a 'text' key. A Blob should have 'mime_type' and 'data' keys. Got keys: ['role', 'content']"

Other

  • there seems to be more bugs related to Gemini's usage downstream. I'll try to find more of them after since I'm also using gemini :D

@geohotstan geohotstan changed the title Fix/gemini keys move process_message inside BaseLLM Mar 17, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 17, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 82.09%. Comparing base (e40fc66) to head (f332531).

Files Patch % Lines
metagpt/provider/google_gemini_api.py 28.57% 5 Missing ⚠️
metagpt/provider/base_llm.py 71.42% 4 Missing ⚠️
metagpt/provider/openai_api.py 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1021      +/-   ##
==========================================
+ Coverage   81.85%   82.09%   +0.24%     
==========================================
  Files         246      246              
  Lines       13725    13732       +7     
==========================================
+ Hits        11234    11273      +39     
+ Misses       2491     2459      -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@geohotstan geohotstan marked this pull request as ready for review March 18, 2024 06:43
@garylin2099
Copy link
Collaborator

A good point. Also @better629 please review gemini part of code.

@garylin2099 garylin2099 requested a review from better629 March 19, 2024 02:23
@garylin2099
Copy link
Collaborator

pre-commit checks failed, suggesting format issues. Please check https://docs.deepwisdom.ai/main/en/guide/contribute/contribute_guide.html#before-submission to format the code.

@better629
Copy link
Collaborator

@geohotstan
The DI support GPT so far, and need further checking with gemini. And process_message has also been used in https://github.com/geekan/MetaGPT/blob/main/metagpt/actions/di/write_analysis_code.py.

So, let do it step-by-step

  1. Can you run examples/llm_hello_world.py with gemini successful? to check if a gemini provider it-self problem.
  2. the original process_message is used in openai provider under aask_code which is not used in gemini
  3. Do a deeper coding, can you to check gemini support tool like openai, so we can support DI full functionality using gemini.

@geohotstan geohotstan marked this pull request as draft March 19, 2024 13:09
@iorisa
Copy link
Collaborator

iorisa commented Mar 20, 2024

process_message has been moved to provider foloder and rename to format_msg
fixbug #1058

Your approach to the modification is correct.
However, LLM has a set of functions for formatting messages, so I renamed process_message to format_msg to correspond with the existing function names:

class BaseLLM(ABC):
    ...

    def _user_msg(self, msg: str, images: Optional[Union[str, list[str]]] = None) -> dict[str, Union[str, dict]]:
        if images:
            # as gpt-4v, chat with image
            return self._user_msg_with_imgs(msg, images)
        else:
            return {"role": "user", "content": msg}

    def _user_msg_with_imgs(self, msg: str, images: Optional[Union[str, list[str]]]):
        """
        images: can be list of http(s) url or base64
        """
        if isinstance(images, str):
            images = [images]
        content = [{"type": "text", "text": msg}]
        for image in images:
            # image url or image base64
            url = image if image.startswith("http") else f"data:image/jpeg;base64,{image}"
            # it can with multiple-image inputs
            content.append({"type": "image_url", "image_url": url})
        return {"role": "user", "content": content}

    def _assistant_msg(self, msg: str) -> dict[str, str]:
        return {"role": "assistant", "content": msg}

    def _system_msg(self, msg: str) -> dict[str, str]:
        return {"role": "system", "content": msg}

    def format_msg(self, messages: Union[str, Message, list[dict], list[Message], list[str]]) -> list[dict]:
        """convert messages to list[dict]."""
        from metagpt.schema import Message

        if not isinstance(messages, list):
            messages = [messages]

        processed_messages = []
        for msg in messages:
            if isinstance(msg, str):
                processed_messages.append({"role": "user", "content": msg})
            elif isinstance(msg, dict):
                assert set(msg.keys()) == set(["role", "content"])
                processed_messages.append(msg)
            elif isinstance(msg, Message):
                processed_messages.append(msg.to_dict())
            else:
                raise ValueError(
                    f"Only support message type are: str, Message, dict, but got {type(messages).__name__}!"
                )
        return processed_messages

    def _system_msgs(self, msgs: list[str]) -> list[dict[str, str]]:
        return [self._system_msg(msg) for msg in msgs]

    def _default_system_msg(self):
        return self._system_msg(self.system_prompt)

@geohotstan
Copy link
Author

other fix merged.

@geohotstan geohotstan closed this Mar 21, 2024
@geohotstan geohotstan deleted the fix/gemini_keys branch April 18, 2024 18:21
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.

用gemini作为LLM,运行数据解析器中的data_visualization.py示例报错
5 participants