imbajin commented on code in PR #359:
URL: https://github.com/apache/hugegraph-ai/pull/359#discussion_r3360277074
##########
hugegraph-llm/src/hugegraph_llm/demo/rag_demo/vector_graph_block.py:
##########
@@ -270,6 +270,12 @@ def create_vector_graph_block():
graph_data_btn0 = gr.Button("Clear Graph Data", size="sm")
vector_import_bt = gr.Button("Import into Vector",
variant="primary")
+ graph_split_type = gr.Dropdown(
Review Comment:
⚠️ **Persist the selected split type**
Evidence: the dropdown is wired into `extract_graph`, but the existing
`store_prompt()` call only saves `doc`, `schema`, and `example_prompt`; reload
also only restores those fields, and `BasePromptConfig.save_to_yaml()` has no
split-type field.
Impact: after reload, a user who selected `paragraph` or `sentence` silently
falls back to `document`, so the next extraction can run with different
chunking than the UI state they expected.
Requested fix: save and reload this split type through the prompt config
path, or make the control explicitly transient. A prompt-config round-trip test
would cover the regression.
##########
hugegraph-llm/src/hugegraph_llm/operators/document_op/chunk_split.py:
##########
@@ -57,7 +62,7 @@ def _get_text_splitter(self, split_type: str):
).split_text
if split_type == SPLIT_TYPE_SENTENCE:
return RecursiveCharacterTextSplitter(chunk_size=50,
chunk_overlap=0, separators=self.separators).split_text
Review Comment:
‼️ **Implement real sentence splitting**
Evidence: with this implementation, `ChunkSplit("Alpha sentence one. Beta
sentence two. Gamma sentence three.", "sentence", "en")` returns `["Alpha
sentence one. Beta sentence two", ". Gamma sentence three."]`, and one long
sentence is split into 7 chunks by the 50-character limit.
Impact: users selecting `sentence` can get merged sentences or mid-sentence
fragments, which changes extraction boundaries and can distort entity/relation
extraction.
Requested fix: either implement sentence-boundary splitting for this mode,
or rename the option/help text so it describes character-sized chunks instead.
Please add a test that asserts simple punctuation-delimited input is split per
sentence.
##########
hugegraph-llm/src/hugegraph_llm/flows/graph_extract.py:
##########
@@ -19,6 +19,10 @@
from hugegraph_llm.flows.common import BaseFlow
from hugegraph_llm.nodes.document_node.chunk_split import ChunkSplitNode
+from hugegraph_llm.operators.document_op.chunk_split import (
Review Comment:
🧹 **Sort the import block**
Evidence: `uv run --project .. --extra llm --extra dev ruff check
src/hugegraph_llm/flows/graph_extract.py
src/hugegraph_llm/utils/graph_index_utils.py
src/hugegraph_llm/operators/document_op/chunk_split.py
src/tests/document/test_graph_extract_configurable_split.py` fails with `I001
Import block is un-sorted or un-formatted` on this file.
Impact: the PR will fail the repository lint gate even though the targeted
tests pass.
Requested fix: run Ruff import sorting on this file and commit the formatted
import order.
--
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]
