Skip to content

fix(rag): escape $ in GraphRAG file_pattern, fix cache type#699

Open
AmirF194 wants to merge 1 commit into
HKUDS:devfrom
AmirF194:fix/695-graphrag-config-invalid-settings
Open

fix(rag): escape $ in GraphRAG file_pattern, fix cache type#699
AmirF194 wants to merge 1 commit into
HKUDS:devfrom
AmirF194:fix/695-graphrag-config-invalid-settings

Conversation

@AmirF194

Copy link
Copy Markdown

GraphRAG's load_config() treats the whole settings.yaml text as a string.Template and substitutes env vars into it before parsing YAML. Our generated file_pattern regex, .*\.txt$, contains a bare $, which is an invalid placeholder to that pass and raises ValueError: Invalid placeholder in string, before indexing can even start.

Once that is escaped, the next call to fail is graphrag_cache.CacheFactory.create_cache (invoked from GraphRAG's own run_pipeline.py): our cache.type was set to "file", but "file" is a storage type, not a registered cache type. GraphRAG's CacheFactory only registers json/memory/none, so the first indexing run raises ValueError: CacheConfig.type 'file' is not registered in the CacheFactory.

Fix

In deeptutor/services/rag/pipelines/graphrag/config.py:

  • file_pattern: .*\.txt$.*\.txt$$ (escapes the literal $ for string.Template).
  • cache.type: "file""json" (the storage type stays "file"; only the cache backend id was wrong).

Verification

Installed graphrag==3.1.1 (the version this repo pins) in a clean Docker container and drove the real path: write_settings()graphrag.config.load_config.load_config()graphrag_cache.cache_factory.create_cache(), the same call graphrag/index/run/run_pipeline.py:45 makes. Both errors reproduce verbatim on unpatched config.py; both are gone after the fix, and create_cache returns a real JsonCache.

Added a regression test in tests/services/rag/test_graphrag_pipeline.py that exercises this same real path, guarded with pytest.importorskip("graphrag") since the package is not installed in this repo's CI (per the test module's own docstring). Ran it RED against the unpatched file and GREEN after the fix, both with graphrag installed; also ran the full existing test file and the full tests/ suite with graphrag uninstalled (matching CI) to confirm no regressions: 2573 passed, 8 skipped, 3 pre-existing failures unrelated to this change (missing git binary in the minimal container, confirmed by installing it). ruff check and ruff format --check (0.16.0, matching CI's lint job) are clean on both changed files.

Did not reproduce or address the issue's third symptom (uvloop/nest_asyncio event-loop clash) since it is an orthogonal event-loop policy issue, not a config-generation bug, and is not required to fix these two.

Fixes #695

GraphRAG's load_config() runs string.Template substitution on the raw
settings.yaml text before parsing it, so the literal $ in our
file_pattern regex (.*\.txt$) raises "Invalid placeholder". Escape
it as $$.

cache.type was set to "file", which is a storage type, not a
registered cache type (GraphRAG's CacheFactory only knows
json/memory/none); the first indexing run raises
"CacheConfig.type 'file' is not registered in the CacheFactory".
Changed to "json".

Verified both failures and the fix against the real graphrag==3.1.1
package (graphrag.config.load_config.load_config +
graphrag_cache.cache_factory.create_cache, the same call
run_pipeline.py makes), not just a yaml roundtrip. Added a regression
test guarded with pytest.importorskip("graphrag") since the package
is not installed in this repo's CI; ran RED on unpatched config.py and
GREEN after the fix, both with graphrag installed in Docker.

Fixes HKUDS#695
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.

1 participant