fix(rag): escape $ in GraphRAG file_pattern, fix cache type#699
Open
AmirF194 wants to merge 1 commit into
Open
fix(rag): escape $ in GraphRAG file_pattern, fix cache type#699AmirF194 wants to merge 1 commit into
AmirF194 wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GraphRAG's
load_config()treats the wholesettings.yamltext as astring.Templateand substitutes env vars into it before parsing YAML. Our generatedfile_patternregex,.*\.txt$, contains a bare$, which is an invalid placeholder to that pass and raisesValueError: 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 ownrun_pipeline.py): ourcache.typewas set to"file", but"file"is a storage type, not a registered cache type. GraphRAG'sCacheFactoryonly registersjson/memory/none, so the first indexing run raisesValueError: 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$forstring.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 callgraphrag/index/run/run_pipeline.py:45makes. Both errors reproduce verbatim on unpatchedconfig.py; both are gone after the fix, andcreate_cachereturns a realJsonCache.Added a regression test in
tests/services/rag/test_graphrag_pipeline.pythat exercises this same real path, guarded withpytest.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 withgraphraginstalled; also ran the full existing test file and the fulltests/suite withgraphraguninstalled (matching CI) to confirm no regressions: 2573 passed, 8 skipped, 3 pre-existing failures unrelated to this change (missinggitbinary in the minimal container, confirmed by installing it).ruff checkandruff 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_asyncioevent-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