Skip to content

fix: detect ainsert signature instead of silently swallowing TypeError#251

Closed
peterCheng123321 wants to merge 1 commit intoHKUDS:mainfrom
peterCheng123321:fix/244-silent-ainsert-failure
Closed

fix: detect ainsert signature instead of silently swallowing TypeError#251
peterCheng123321 wants to merge 1 commit intoHKUDS:mainfrom
peterCheng123321:fix/244-silent-ainsert-failure

Conversation

@peterCheng123321
Copy link
Copy Markdown

@peterCheng123321 peterCheng123321 commented Apr 21, 2026

Summary

Fixes #244.

insert_text_content_with_multimodal_content in utils.py was wrapping the entire lightrag.ainsert() call in a bare except Exception that logged a hint and returned silently. When LightRAG's ainsert does not accept a multimodal_content keyword argument (i.e. the standard PyPI release rather than the RAGAnything branch), the TypeError was swallowed — meaning:

  • No text was indexed
  • No kv_store_doc_status entry was written
  • adelete_by_doc_id returned 404

Fix: use inspect.signature to check upfront whether ainsert accepts multimodal_content. If it does, call with the full argument set. If not, emit a visible logger.warning and fall back to text-only insertion so the document is still indexed and doc_status is correctly written. Real errors (network failures, LLM errors, etc.) now propagate instead of being silently dropped.

Test plan

  • Run process_document_complete() against the standard PyPI LightRAG release — confirm kv_store_doc_status is created and adelete_by_doc_id succeeds
  • Run against the RAGAnything LightRAG branch — confirm full multimodal path still works
  • Confirm any real ainsert errors (e.g. bad API key) now surface as exceptions

insert_text_content_with_multimodal_content was catching all exceptions
and logging a hint, so a TypeError from an incompatible LightRAG version
(ainsert lacking multimodal_content) caused the entire insert to be
silently dropped — no text indexed, no doc_status written, and
adelete_by_doc_id returning 404.

Use inspect.signature to check upfront whether ainsert accepts
multimodal_content, and fall back to text-only insertion with a visible
warning if it does not. Real errors now propagate instead of being
swallowed.

Fixes HKUDS#244

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@LarFii
Copy link
Copy Markdown
Collaborator

LarFii commented Apr 25, 2026

Thanks for the fix. I checked this against the current LightRAG ainsert signature, and I think there is still a blocker before this can be merged.

The fallback path only checks whether ainsert supports multimodal_content, but it still passes scheme_name=scheme_name when multimodal_content is not supported. Current upstream/PyPI-style LightRAG ainsert also does not accept scheme_name, so callers that pass a non-empty scheme will still hit TypeError: unexpected keyword argument 'scheme_name' and the document/status creation path can still fail.

Could you please update the compatibility logic to filter all optional extension kwargs against the actual ainsert signature, or omit unsupported kwargs entirely in the fallback path? #255 has a more complete version of that direction, so this PR may also be superseded by that one once the remaining doc_status semantics are resolved.

@LarFii
Copy link
Copy Markdown
Collaborator

LarFii commented May 6, 2026

@peterCheng123321 Thanks for the quick turnaround on this. Closing in favor of #255, which has just merged into main. The complete fix for #244 lives there:

  • filters all optional ainsert kwargs (multimodal_content and scheme_name) against the actual LightRAG signature, so old PyPI builds that don't accept either of them no longer hit TypeError
  • bootstraps doc_status records up front so adelete_by_doc_id works even when LightRAG's ainsert falls back to text-only
  • adds compatibility multimodal completion state in a separate KV namespace for older LightRAG versions whose DocProcessingStatus schema rejects multimodal_processed
  • ships regression tests covering the empty-ainsert-signature case this PR was originally targeting

The signature-detection idea you proposed here ended up being the core mechanism in #255, so your direction was right. Really appreciate the contribution.

@LarFii LarFii closed this May 6, 2026
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]: No doc_status created after process_document_complete

3 participants