Skip to content

fix: remove trailing comma that wrapped triples result in tuple in _invoke#745

Open
octo-patch wants to merge 1 commit intoOpenSPG:masterfrom
octo-patch:fix/issue-743-triples-extraction-tuple-bug
Open

fix: remove trailing comma that wrapped triples result in tuple in _invoke#745
octo-patch wants to merge 1 commit intoOpenSPG:masterfrom
octo-patch:fix/issue-743-triples-extraction-tuple-bug

Conversation

@octo-patch
Copy link
Copy Markdown

Fixes #743

Problem

In the synchronous _invoke method of SchemaFreeExtractor, the result of triples_extraction was accidentally wrapped in a 1-element tuple due to a trailing comma:

# Before (buggy)
triples = (self.triples_extraction(passage, filtered_entities),)

When assemble_sub_graph_with_triples iterates for tri in triples:, it received the entire list of triples as a single element rather than iterating individual triples. The check if tri is None or len(tri) != 3: then always skipped this element (since the list length is not 3), causing all relationships to be silently dropped in synchronous extraction mode.

The async _ainvoke path was unaffected because it uses asyncio.gather, which unpacks results correctly into triples directly.

Solution

Remove the trailing comma so triples holds the list returned by triples_extraction directly:

# After (fixed)
triples = self.triples_extraction(passage, filtered_entities)

Testing

The fix aligns the synchronous _invoke behavior with the async _ainvoke path, which already worked correctly. The one-character change (removing the trailing comma) is the minimal fix needed.

…OpenSPG#743)

In the synchronous _invoke method of SchemaFreeExtractor, the result of
triples_extraction was accidentally wrapped in a 1-element tuple due to a
trailing comma:

    triples = (self.triples_extraction(passage, filtered_entities),)

When assemble_sub_graph_with_triples iterates for tri in triples, it
would receive the whole list of triples as a single element (not individual
triples), causing the len(tri) != 3 check to discard every relationship.
The async _ainvoke path was unaffected because it used asyncio.gather.

Fix: remove the trailing comma so triples holds the list directly.
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.

[Bug] [Module Name] Bug title

1 participant