feat(extraction): add experimental runtime with review and concurrent chunks - #372
feat(extraction): add experimental runtime with review and concurrent chunks#372UIengF wants to merge 10 commits into
Conversation
84482e0 to
1a56bfa
Compare
Change-Id: Ied8edec3c6dd9be2ec7110f60a2fa92754660977
1a56bfa to
6545970
Compare
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: the biggest simplification here is to land only the slice a real caller exercises: right now 2,612 lines of runtime source ship inside src/hugegraph_llm/ with no importer outside the package's own tests, one fixture domain, one replay transport, and a Protocol (ProviderAdapterV1) that nothing is typed against. Evidence: git grep -n extraction_runtime 6545970 -- ':!*extraction_runtime*' ':!*src/tests*' returns only docs and MANIFEST.in; the package docstring says "Dormant", docs/extraction-runtime.md says "The package has no production caller", and test_production_modules_do_not_import_the_dormant_runtime asserts it; the only ExtractionBundleV1 and ProviderTransportV1 implementations in the tree are the inventory fixture and the scripted test doubles; litellm and tenacity are already declared dependencies in hugegraph-llm/pyproject.toml and already used in models/llms/litellm.py.
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| """Dormant experimental extraction runtime. |
There was a problem hiding this comment.
docs/extraction-runtime.md says "The package has no production caller", and test_packaging_compatibility.py::test_production_modules_do_not_import_the_dormant_runtime enforces it.
That is 2,612 lines landing inside the distributed package src/hugegraph_llm/, so pip install hugegraph-llm ships a dormant subsystem plus a 300-line fixture domain (conformance/inventory.py) and a replay test double.
The tightest example of the pattern is the packaged descriptor: resources/runtime-contract-v1.json (25 lines), resources/__init__.py, v1/resources.py with its schema-version validation, and a new MANIFEST.in recursive-include. That is roughly 71 lines and a packaging rule whose only consumer is a test asserting the JSON still equals the RUNTIME_CONTRACT literal in v1/fingerprint.py that it was copied from.
Requested change: land the slice a caller exercises. Pick the one integration you actually want (GraphExtractFlow routing a single extraction through the review/repair loop), and ship the engine, graph state, budget accounting, and terminal resolution that path uses. Fingerprint layers, semantic manifests, adaptation records, the packaged descriptor, and the provider dialect can follow the first code that reads them. If the prototype must land whole, put it in a top-level examples/ rather than src/hugegraph_llm/, so users don't install a subsystem the project itself does not call.
| ) | ||
|
|
||
|
|
||
| class ProviderAdapterV1(Protocol): |
There was a problem hiding this comment.
ProviderAdapterV1. It is declared here and re-exported in provider/__init__.py, and that is every reference to the name in the tree: no parameter is annotated with it, nothing accepts it, and InventoryBundleV1.__init__ builds a concrete ProviderDialectV1 directly. ProviderDialectV1.plan does match it structurally, which is rather the point: the satisfying class is one file over, so declaring the Protocol buys nothing today.
The other two are barely better. ProviderTransportV1 (line 239) is satisfied only by ReplayProvider and the scripted doubles in the tests; ExtractionBundleV1 (v1/engine.py:53) only by the inventory fixture and test_engine.py's ScriptedInventoryBundle.
Requested change: delete ProviderAdapterV1 and its export, and let InventoryBundleV1 name ProviderDialectV1 directly. Add a Protocol back the day a second implementation exists. (Leaving ExtractionEngineV1 alone here on purpose: your own test_dependency_guard.py forbids v1/ importing conformance, so the engine cannot name the concrete bundle without that guard going too.)
|
|
||
| contract = "provider-dialect/v1" | ||
|
|
||
| def plan( |
There was a problem hiding this comment.
hugegraph-llm/pyproject.toml declares litellm, and models/llms/litellm.py already wraps it. litellm.get_supported_openai_params(model=...) reports which parameters a provider accepts and litellm.drop_params = True drops the rest: the same kept / dropped / downgraded decision this 171-line dialect and the ProviderCapabilitiesV1 / AdaptationDecisionV1 / AdaptationRecordV1 dataclasses hand-roll. The difference cuts against the hand-rolled version, because litellm knows each provider's real capabilities, whereas ProviderCapabilitiesV1 makes every caller declare them by hand and get the optional ones quietly wrong.
Related: retry_policy and timeout_seconds are validated, copied into the effective payload, and folded into the digest, but no transport reads either. Live transports being application work is the stated design, so the simplification is to add these two fields alongside the transport that honours them. Retry in this repo is tenacity, already a dependency and already used in models/llms/litellm.py.
Requested change: drop provider/dialect.py and the capability/adaptation dataclasses, and build the request through the existing LiteLLMClient when a live transport lands. If a record of dropped parameters is genuinely needed, it is a log line next to that call, not a contract layer.
| result = ExtractionEngineV1().run(bundle=bundle, chunk=chunk, control=control) | ||
| return ChunkRunResultV1(chunk=chunk, result=result) | ||
|
|
||
| with ThreadPoolExecutor(max_workers=max_workers) as executor: |
There was a problem hiding this comment.
executor.map. Take away the license header, the ChunkRunResultV1 dataclass and the 10-line docstring, and what is left is a four-line run_chunk closure over prepare and ExtractionEngineV1().run, then:
with ThreadPoolExecutor(max_workers=max_workers) as executor:
return tuple(executor.map(run_chunk, chunks))The docstring, plus roughly twenty more lines in docs/extraction-runtime.md, partly restates ThreadPoolExecutor's own documented behaviour: results in input order, running tasks finish before the pool closes, the whole batch held in memory. ChunkRunResultV1 pairs a chunk with the result of that chunk, which the caller already has.
Requested change: delete v1/batch.py along with its v1/__init__.py re-exports and the 195-line test_batch.py, and put those two lines in the docs instead: a caller who wants a batch writes them and keeps its own result type. If a batch helper does earn a place later, note the repo already runs every extraction flow through pycgraph.GPipeline (flows/graph_extract.py), which owns element ordering and parallel execution.
| assert tuple(resource["terminal_kinds"]) == ("final", "candidate", "blocked", "failed") | ||
|
|
||
|
|
||
| def test_graph_extract_route_and_defaults_remain_unchanged() -> None: |
There was a problem hiding this comment.
test_graph_extract_route_and_defaults_remain_unchanged string-matches an exact decorator line from api/graph_extract_api.py, and test_existing_scheduler_still_owns_graph_extract_flow (line 49) string-matches an import line and two expressions from flows/scheduler.py. Neither file imports the runtime, so nothing here can regress it, but wrapping a long line or reordering a decorator argument in either file breaks this PR's tests for reasons unrelated to extraction.
test_production_modules_do_not_import_the_dormant_runtime (line 57) has the mirror-image problem: it is a test asserting the feature is unused, so it has to be deleted the day the feature is used.
Requested change: drop the string-matching assertions and delete that last test. Keep lines 42-46, the four GraphExtractRequest default checks, which are behavioural and survive reformatting. test_dependency_guard.py already enforces the direction that matters (the runtime not reaching into api, flows, nodes, operators or pyhugegraph), and it does so by walking imports rather than matching source strings. Keep that one.
Overview
Add an experimental, domain-neutral extraction runtime for already-normalized text chunks. Domain implementations provide extraction and quality rules; the runtime coordinates validation, review, repair, and final decisions. It supports both single-chunk execution and bounded concurrent batches.
Main capabilities
Integration boundary
The runtime is an experimental Python API within
hugegraph-llm, targeting Python 3.10 and 3.11. Existing extraction routes and production callers retain their current behavior. Live model transports, document preparation, persistence, cross-chunk merging, and graph publication remain application integration work. The versioned interfaces are experimental rather than a stable public API commitment.Verification
BOLT finalandNUT finalwithout model credentials or a running HugeGraph server.Full-repository Ruff formatting and lint checks pass with Ruff 0.15.18, the version used by the last successful main-branch Ruff workflow. The development dependency is pinned to that version so the existing checks remain reproducible across installations. Live-model quality and external-service integration were not tested locally as part of this prototype.