Copilot commented on code in PR #269:
URL:
https://github.com/apache/incubator-hugegraph-ai/pull/269#discussion_r2218814994
##########
hugegraph-llm/src/hugegraph_llm/config/models/base_prompt_config.py:
##########
@@ -29,6 +29,13 @@
yaml_file_path = os.path.join(os.getcwd(), "src/hugegraph_llm/resources/demo",
F_NAME)
+class LiteralStr(str): pass
Review Comment:
The LiteralStr class lacks documentation explaining its purpose for YAML
literal string representation. Adding a docstring would improve code
maintainability.
```suggestion
class LiteralStr(str):
"""
A subclass of `str` used to represent YAML literal strings.
This class is associated with a custom YAML representer
(`literal_str_representer`)
that ensures strings of this type are serialized using the literal style
(`|`).
"""
```
##########
hugegraph-llm/src/hugegraph_llm/config/prompt_config.py:
##########
@@ -424,4 +426,6 @@ class PromptConfig(BasePromptConfig):
{user_scenario}
## Your Generated "Graph Extract Prompt Header":
+## Language Requirement:
+Please generate the prompt in {language} language.
Review Comment:
The comment section '## Language Requirement:' appears to be incomplete. It
should include more detailed documentation about how the language parameter
affects prompt generation and what values are expected.
```suggestion
Please generate the prompt in {language} language.
# The `language` parameter specifies the desired output language for the
generated prompt.
# It affects the language used in the generated text, including
instructions, examples, and any other content.
# Expected values for the `language` parameter include standard language
names (e.g., "English", "French") or language codes (e.g., "en", "fr").
# Ensure that the provided `language` value is supported by the system to
avoid errors or unexpected behavior.
```
##########
hugegraph-llm/src/hugegraph_llm/config/models/base_prompt_config.py:
##########
@@ -61,71 +69,74 @@ def ensure_yaml_file_exists(self):
# Load existing values from the YAML file into the class
attributes
for key, value in data.items():
setattr(self, key, value)
+
+ # Check if the language in the .env file matches the language in
the YAML file
+ env_lang = self.llm_settings.language.lower() if hasattr(self,
'llm_settings') and self.llm_settings.language else 'en'
+ yaml_lang = data.get('_language_generated', 'en').lower()
+
+ if env_lang.strip() != yaml_lang.strip():
+ log.warning(
+ f"Prompt was changed '.env' language is '{env_lang}', "
+ f"but '{F_NAME}' was generated for '{yaml_lang}'. "
+ f"Regenerating the prompt file..."
+ )
+ if self.llm_settings.language.lower() == "cn":
+ self.answer_prompt = self.answer_prompt_CN
+ self.extract_graph_prompt = self.extract_graph_prompt_CN
+ self.gremlin_generate_prompt =
self.gremlin_generate_prompt_CN
+ self.keywords_extract_prompt =
self.keywords_extract_prompt_CN
+ self.doc_input_text = self.doc_input_text_CN
+ else:
+ self.answer_prompt = self.answer_prompt_EN
+ self.extract_graph_prompt = self.extract_graph_prompt_EN
+ self.gremlin_generate_prompt =
self.gremlin_generate_prompt_EN
+ self.keywords_extract_prompt =
self.keywords_extract_prompt_EN
+ self.doc_input_text = self.doc_input_text_EN
else:
self.generate_yaml_file()
log.info("Prompt file '%s' doesn't exist, create it.",
yaml_file_path)
Review Comment:
The language selection logic is duplicated in lines 83-94 and 126-137. This
should be extracted into a separate method to reduce code duplication and
improve maintainability.
```suggestion
self._set_language_prompts()
else:
self.answer_prompt = self.answer_prompt_EN
self.extract_graph_prompt = self.extract_graph_prompt_EN
self.gremlin_generate_prompt = self.gremlin_generate_prompt_EN
self.keywords_extract_prompt = self.keywords_extract_prompt_EN
self.doc_input_text = self.doc_input_text_EN
```
##########
hugegraph-llm/src/hugegraph_llm/config/models/base_prompt_config.py:
##########
@@ -61,71 +69,74 @@ def ensure_yaml_file_exists(self):
# Load existing values from the YAML file into the class
attributes
for key, value in data.items():
setattr(self, key, value)
+
+ # Check if the language in the .env file matches the language in
the YAML file
+ env_lang = self.llm_settings.language.lower() if hasattr(self,
'llm_settings') and self.llm_settings.language else 'en'
+ yaml_lang = data.get('_language_generated', 'en').lower()
+
+ if env_lang.strip() != yaml_lang.strip():
+ log.warning(
+ f"Prompt was changed '.env' language is '{env_lang}', "
+ f"but '{F_NAME}' was generated for '{yaml_lang}'. "
+ f"Regenerating the prompt file..."
+ )
+ if self.llm_settings.language.lower() == "cn":
Review Comment:
The hardcoded string comparison 'cn' doesn't match the Literal type
definition which uses 'CN' (uppercase). This could cause the Chinese prompts to
never be selected since LLMConfig defines language as Literal['EN', 'CN'].
```suggestion
if self.llm_settings.language == "CN":
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
