Repository navigation
context-graph-eval: GLiNER2 reconciliation backend, embedding-concurrency fix, #324 diagnosis - #340
Merged
Merged
Conversation
GLiNER2Backend (local, LLM-free entity extraction, added in #330/#332) was only reachable from unstructured2graph directly -- sessions-graph and context-graph-eval hardcoded LightRAGBackend(lightrag_wrapper), so no LongMemEval question could ever run against a GLiNER2-built graph. That blocked comparing the two on real eval questions rather than just the existing deterministic extraction_quality.py gold-corpus comparison. - SessionsGraph.reconcile_session gains an optional extraction_backend parameter, defaulting to LightRAGBackend(lightrag_wrapper) -- the original, only behavior before this existed, so every current caller is unaffected. lightrag_wrapper stays required either way: narrative summarization (the session's Episode) is a generative task GLiNER2 cannot do at all, so it always runs through the LightRAG wrapper's own LLM regardless of which backend extracts entities. - context_graph_eval.reconcile.reconcile_batch gains extraction_backend: "lightrag" (default) | "gliner2". GLiNER2 mode reconciles sessions one at a time via reconcile_session rather than through reconcile_sessions_batch: map #322's batch/queue pipeline exists to give LightRAG's own worker pool more than one document at a time, which is meaningless for a backend with no shared busy-lock to fan out over. - --extraction-backend {lightrag,gliner2} on the run CLI, threaded through RunPlan. RunMeta now records it, and compare() refuses across a mismatch the same way it already refuses across a judge, agent model, or tokenizer mismatch -- a LightRAG-built and a GLiNER2-built graph are different systems under test. Verified: sessions-graph 70 passed/1 skipped (1 new), context-graph-eval 189 passed/5 skipped (4 new: unknown-backend validation, gliner2 mode reconciles per-session not via the batch pipeline, lightrag mode unaffected, backend mismatch refused in compare()), unstructured2graph 100 passed/9 skipped (untouched). Full-workspace ty check and ruff clean.
… cover it Measured live: a real 10-session batch failed 9/10 with "Embedding func: Worker execution timeout after 60s". _resolve_reconciliation_tuning raises MAX_PARALLEL_INSERT and MAX_ASYNC_LLM (map #322) for the OpenAI-backed extraction LLM, which genuinely benefits from more concurrent requests to a rate-limited remote API. But LightRAG's embedding calls run through their own separate limit, EMBEDDING_FUNC_MAX_ASYNC (default 8) and timeout, EMBEDDING_TIMEOUT (default 30s -> a 60s worker-kill, doubled internally) -- untouched by that tuning. With MAX_PARALLEL_INSERT=16 now putting many documents in flight at once, up to 8 of them fire embedding calls concurrently against the SAME local, CPU-bound bge-m3 model (#331). A shared local model does not parallelize like a remote API scales -- concurrent calls contend for one resource, so throughput gets worse, not better, as concurrency rises. EMBEDDING_FUNC_MAX_ASYNC is now lowered to 2 (serializing most embedding work, which is what a single local model can actually sustain) and EMBEDDING_TIMEOUT raised to 120s as headroom, both via the same eval-scoped setdefault convention as the existing knobs -- an operator's own exported value, or production sessions-graph reconciliation (which sets none of this), stay untouched. Verified: context_graph_eval/tests/test_reconcile.py 8 passed (2 new). Full workspace ruff/ty clean. Aside, not fixed here: running test_e2e_reconcile.py's full file with a real OPENAI_API_KEY (previously never exercised in this repo's history -- always skipped for lack of a key) surfaced a separate, pre-existing bug: LightRAG's global shared_storage lock gets bound to one test's event loop and then reused by a later @pytest.mark.asyncio test's own loop, raising "bound to a different event loop". Reproduced in isolation from this change (each failing test passes alone); unrelated to reconciliation tuning.
Standards: - Persisted extraction-backend provenance and validate it on reuse (see Spec below) -- the real bug driving most of this. - SessionsGraph._write_completed now records which backend class actually extracted a session's entities (None when nothing was extracted -- a real, distinct state, not an omission). reconcile_session derives it from whichever backend it used; reconcile_sessions_batch's own completion path is unconditionally LightRAG (map #322's batch pipeline has no other backend), so it's a literal there. - Converted test_reconcile.py's two reconcile_batch strategy tests off a hand-built SessionsGraph fake onto a real SessionsGraph/ActionsGraph against the managed test Memgraph -- this repo's testing policy prefers a real instance of another package's class over exactly this kind of stand-in. Only the LLM boundary is stubbed (free, deterministic, unstructured2graph's own established pattern) and GLiNER2's underlying model is faked via the class's own documented no-gliner2-install escape hatch, so neither needs OPENAI_API_KEY or a real `gliner2` package. - extraction_backend is now `Literal["lightrag", "gliner2"]` (ExtractionBackendName) through reconcile.py/runner.py, not a bare str -- a typo at a call site we control is now a type error, not a ValueError three calls later. Left RunMeta's own field as plain str deliberately: it is a deserialization boundary (loaded from arbitrary saved JSON, possibly from an older code version), the same reason judge_model/tokenizer are also untyped strings there. - De-duplicated reconcile_batch's per-branch result accounting and progress printing into two small shared helpers. - Fixed reconcile_session's docstring (still said "LightRAG entity-extraction pipeline" unconditionally) and the sessions-graph README (prose and the API table both omitted extraction_backend entirely). Fixed unstructured2graph/evals/README.md, which explicitly claimed GLiNER2 was not wired into context-graph-eval's pipeline -- it now is. Documented --extraction-backend and the provider-qualified --judge-model/--agent-model spec (#329) in context-graph/eval/README.md, whose own example still used the pre-#329 bare-model-id form. Reviewed, not changed: commit subjects without a "(#PR)" suffix. Checked against this repo's own merge history (e.g. ee1a141, 4789dc3) -- GitHub's squash-merge appends "(#<PR>)" to the PR title automatically; AGENTS.md's own worked example carries no such suffix either. A pre-merge commit cannot know its future PR number, so there is nothing to add here. Spec: - High: --skip-reconcile could save false backend provenance. RunMeta recorded whatever --extraction-backend said on the command line, but --skip-reconcile never rebuilds anything -- reusing a LightRAG-built graph with --extraction-backend gliner2 would record "gliner2" despite every entity in the graph coming from LightRAG, letting compare() either wrongly refuse two runs that used the same real backend or wrongly compare two that didn't. runner._require_reconciled now checks the persisted backend above against what this run claims, refusing on a real mismatch the same way it already refuses on missing/pending sessions. None (no reconcilable content) is excluded from the check, not treated as a mismatch. - Medium: stale documentation -- covered above. Verified: sessions-graph 70 passed/1 skipped, context-graph-eval 193 passed/5 skipped (2 tests converted to real instances, 3 new: backend mismatch refused, no-recorded-backend not refused, provenance written correctly by both reconciliation strategies). Full-workspace ty check and ruff clean.
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.
Summary
Two independent pieces, both discovered/needed while diagnosing #324 for real:
GLiNER2 as an alternate reconciliation backend -- GLiNER2Backend (local, LLM-free extraction, unstructured2graph: pluggable extraction backends + GLiNER2 #330/unstructured2graph: extraction-quality eval, GLiNER2 vs LightRAG #332) was only reachable from
unstructured2graphdirectly;sessions-graph/context-graph-evalhardcodedLightRAGBackend(lightrag_wrapper), so no LongMemEval question could run against a GLiNER2-built graph.SessionsGraph.reconcile_sessiongains an optionalextraction_backendparam (defaults to the original LightRAG behavior), andcontext_graph_eval.reconcile.reconcile_batch/therunCLI gain--extraction-backend {lightrag,gliner2}. GLiNER2 mode reconciles one session at a time (no batch/queue pipeline to fan out over -- map eval: reconciliation is sequential, and --skip-reconcile cannot actually skip it #322's pipeline is LightRAG-specific).RunMetarecords it andcompare()refuses across a mismatch, same as judge/agent model/tokenizer.Embedding concurrency was never tuned, only the LLM concurrency was -- real run, 9/10 sessions failed with
Embedding func: Worker execution timeout after 60s._resolve_reconciliation_tuningraisesMAX_PARALLEL_INSERT/MAX_ASYNC_LLM(map eval: reconciliation is sequential, and --skip-reconcile cannot actually skip it #322) for the OpenAI-backed extraction LLM, which genuinely benefits from more concurrent requests to a rate-limited remote API. LightRAG's embedding calls run through their own separate limit (EMBEDDING_FUNC_MAX_ASYNC, default 8) and timeout (EMBEDDING_TIMEOUT, default 30s -> 60s worker-kill) -- untouched by that tuning. WithMAX_PARALLEL_INSERT=16now putting many documents in flight, up to 8 fire embedding calls concurrently against the SAME local, CPU-bound bge-m3 model (context-graph-eval: cut reconciliation cost ~14x (map #297) #331) -- which contends with itself instead of parallelizing. Now lowered to 2 (serialize, which a single local model can actually sustain) and the timeout raised to 120s as headroom.Real-world validation
Ran the actual Tier 1 LongMemEval CLI end to end multiple times against the dedicated eval instance, real OpenAI + Anthropic keys, 5 questions /
--max-sessions-per-question 1(~10 unique sessions):Not a verdict on which backend is "better" -- 5 questions is a signal, not a sample size, and this is exactly the smaller-scale comparison this PR exists to make possible for the first time.
#324 diagnosis: corrected
The issue's own hypothesis was "the judge scores identical answers differently." Real testing against a frozen (
--skip-reconcile) graph shows that's not quite it:LLMTestCase(same input/actual_output/expected_output, no retrieval involved) scored by the same judge 5x in a row: perfectly stable, both Anthropic and OpenAI (eval: the judge is hardcoded to Anthropic, but the decision was "a different provider from the pipeline" #329's provider resolution is what made testing the OpenAI side possible at all).ceb54acb's retrieved answer text was byte-identical across 6 independent runs, and itscoveredstatus still flipped 0.0/0.0/1.0/0.917/0.971/0.0.LIMIT/ORDER BY/column-aliasing choices call to call -- a well-documented limitation of hosted LLM APIs, not something fixable in our code). That changesretrieval_context's content and literal rendering run to run against the exact same frozen graph, which flipsContextualRecallMetric's score even when the final natural-language answer never changes.So: the judge is stable; retrieval itself is irreducibly non-deterministic upstream of it. None of #324's judge-side mitigations (swap provider, move the 0.7 gate, median-of-N judge calls on the same input) address that -- the one that does is already on its own list: report a per-question pass rate over repeats instead of a single pass/fail, making the instability visible rather than averaging it into a headline. Not implemented in this PR -- it changes the eval loop's reporting shape, which is a design decision worth its own sign-off rather than bundling into this diff.
Test plan
sessions-graph: 70 passed / 1 skipped (1 new)context-graph-eval: 193 passed / 5 skipped (6 new: extraction-backend validation, gliner2/lightrag reconciliation branching, backend-mismatch refusal incompare(), embedding-tuning knobs)unstructured2graph: 100 passed / 9 skipped (untouched)ty check .andruff check/format --check: cleanAside, not fixed here (unrelated to this PR, filed as its own consideration): running
test_e2e_reconcile.py's full file with a real key for the first time in this repo's history surfaced a pre-existing bug -- LightRAG's globalshared_storagelock gets bound to one test's event loop and then reused by a later test's own loop, raising "bound to a different event loop." Reproduces in isolation from these changes.