Skip to content

fix: unpack community results from semaphore_gather in add_episode#1486

Open
just-jeb wants to merge 3 commits into
getzep:mainfrom
just-jeb:fix/community-update-semaphore-unpack
Open

fix: unpack community results from semaphore_gather in add_episode#1486
just-jeb wants to merge 3 commits into
getzep:mainfrom
just-jeb:fix/community-update-semaphore-unpack

Conversation

@just-jeb

@just-jeb just-jeb commented May 12, 2026

Copy link
Copy Markdown

Summary

Fixes a ValueError: not enough values to unpack (expected 2, got N) crash in add_episode when update_communities=True.

Type of Change

  • Bug fix

Objective

semaphore_gather returns a list of results — one (communities, edges) tuple per coroutine (one per extracted node). The previous code tried to unpack that list directly as two variables:

communities, community_edges = await semaphore_gather(
    *[update_community(...) for node in nodes],
    max_coroutines=self.max_coroutines,
)

This raises ValueError: not enough values to unpack (expected 2, got N) for any episode that extracts at least one node, making add_episode with update_communities=True entirely broken.

Fix: collect into a results list, then flatten:

community_results = await semaphore_gather(...)
communities = [c for r in community_results for c in r[0]]
community_edges = [e for r in community_results for e in r[1]]

Testing

  • Unit tests added/updated (tests/test_community_update_unpack.py)
  • Integration tests added/updated
  • All existing tests pass

Two unit tests:

  1. test_semaphore_gather_community_results_flatten — verifies the fixed flatten logic correctly collects communities and edges across multiple nodes
  2. test_semaphore_gather_unpack_raises_without_fix — demonstrates the old pattern raises ValueError, confirming the bug existed

Breaking Changes

  • This PR contains breaking changes

Checklist

  • Code follows project style guidelines (make lint passes)
  • Self-review completed
  • No secrets or sensitive information committed

Related Issues

Reproduces on graphiti-core==0.29.0. Verified fix locally.

@just-jeb

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

just-jeb added 2 commits June 9, 2026 07:29
When update_communities=True, semaphore_gather returns a list of
(communities, edges) tuples — one per extracted node. The previous
code tried to unpack this list directly as two values:

    communities, community_edges = await semaphore_gather(...)

This raises ValueError: not enough values to unpack (expected 2, got N)
for any episode that extracts at least one node.

Fix: collect the results list, then flatten into the two expected lists.
@danielchalef danielchalef force-pushed the fix/community-update-semaphore-unpack branch from 0ebdd0b to ef3ae46 Compare June 9, 2026 14:30
@zep-cla-assistant

Copy link
Copy Markdown
Contributor


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. For privacy information, see our Privacy Notice. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA behalf on myself, e-mail: [email protected]

or

I have read the CLA Document and I hereby sign the CLA behalf of my company, e-mail: [email protected]

Signature is valid for 6 months.


This bot will be retriggered when the Contributor License Agreement comment has been provided. Posted by the CLA Assistant Lite bot.

@danielchalef

Copy link
Copy Markdown
Member

Maintainer note: rebased this onto current main (the fix still applies cleanly at graphiti.py:1185) and verified the regression test passes. Selecting this as the canonical fix for the update_community / semaphore_gather unpack bug — it's the most complete of the duplicates and the only one with a regression test. Closing #1421, #1254, and #1085 as superseded by this PR. Thanks!

The test file had two module-level docstrings; ruff parses the second as a
statement, demoting the imports below it (6x E402). Merge them into one
docstring and drop the unused AsyncMock/patch imports (F401). No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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.

2 participants