feat(): remove llm chain for chatopenai

[mattzcarey]

No description provided.

[github-actions]

LOGAF Level 3 - /home/runner/work/GenossGPT/GenossGPT/genoss/llm/hf_hub/base_hf_hub.py

1. The generate_embedding method raises a NotImplementedError with a message indicating it's not used. If this is the case, consider removing the method or explaining why it's necessary in a comment.
2. Add docstrings to the generate_answer and generate_embedding methods to explain their purpose and functionality.

---

LOGAF Level 3 - /home/runner/work/GenossGPT/GenossGPT/genoss/llm/fake_llm.py

1. The generate_embedding method uses a hardcoded size of 128 for the FakeEmbeddings model. Consider making this a parameter or constant to increase flexibility.
2. Add docstrings to the generate_answer and generate_embedding methods to explain their purpose and functionality.

---

LOGAF Level 3 - /home/runner/work/GenossGPT/GenossGPT/genoss/llm/openai/openai_llm.py

1. The api_key is exposed. Consider using a secure method to store and retrieve sensitive data like API keys.
2. Add docstrings to the generate_answer and generate_embedding methods to explain their purpose and functionality.

---

🔒📚🔧

---

Powered by Code Review GPT

[Wirg]

You should probably just add an if with a different display.

Something like :

if msg["role"] == "system":
    st.markdown(f"System: *{msg['content']}*")
    continue

If you really don't want to display it don't but go with a if logic anyway.

[Wirg]

not clear to me why you are doing this.

[mattzcarey]

Fix-ruff did this otherwise it errored. See other comment

[Wirg]

Does it still do the same if you remove TYPE_CHECKING and go with a normal import ?

[Wirg]

I feel like it's a good move to leave LLMChain. This method should probably be outside this class tho and reused through various LLMs.

Another thing. Python use snake_case and not CamelCase or pascalCase for functions.

Finally you may want to use python list comprehension.

  return [
    ChatMessage(content=message.content, role=message.role)
    for message in messages
  ]

[mattzcarey]

Understand and agree most of this but what do you mean by keeping LLM chain?

Calling the LLM directly makes more sense than using a chain IMO. Reduce some latency and remove complexity.

[Wirg]

I feel like it's a good move to leave LLMChain

XD I meant move away from it not keep it. I guess it's french synonymy. :P

[Wirg]

Do you have any reason to do that TYPE_CHECKING thing ?
I think we should avoid this unless we have a circular import issue.

[mattzcarey]

fix-ruff did this. Otherwise ruff errored. Happy if you have another option?

[Wirg]

I think you probably should just do the import without the TYPE_CHECKING.