feat: add build-kg --threads to control parallel fullmap reads - #101
Merged
Conversation
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SkyeAv
added a commit
that referenced
this pull request
Aug 14, 2026
Cut 12.0.0 and bump the package version in pyproject.toml, uv.lock, and CITATION.cff. Major, not minor: three breaking changes accumulated since 11.0.0 — the agent now builds into a caller-owned graph config instead of maintaining an internal registry (the graph_registry module, <state-dir>/graph.yaml, and the rebuild-agent-graph subcommand are gone), the QC embedding stage migrated from BioBERT to SapBERT with the model cache moving to .tablassert/sapbert/ (#93), and build-fullmap's prebuilt extraction moved into the Rust extension with force-build-contract validation before install (#97). The changelog needed restructuring, not just promoting: #93 and #97 appended their entries inside the 11.0.0 section, so the SapBERT, abbreviation-stage, and prebuilt-extraction entries are moved into 12.0.0 where they belong, and the caller-owned-graph-config change landed with no entry at all, so its breaking change (the removed --fullmap flag and rebuild-agent-graph subcommand, in-place graph mutation, orphaned registries, and the graph=/graph_path= API) is written up here. Also ships in this release: approval_ids as a curated pass-through edge field, build-kg --threads for parallel fullmap reads scaling past the 16 record shards (#101), release-mode dropping of zero effect-size edges (#95), the species_context_qualifier derivation fix, and the fix that stops fabricating a supporting study for sections with no publication and nothing to preserve (#99).
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.
build-kggains a--threadsflag for the fullmap reads behind entity resolution — and those reads can finally scale past the 16 RECORDS shard files, because the fan-out no longer stops at one reader per shard.CLI plumbing
build-kgaccepts--threads/-t(src/tablassert/cli.py), mirroringbuild-fullmap --threads; a non-positive value exits 2 before any build work starts, matching the--gepa-threadsgate.threadsflowsbuild_kg→build_pipeline→build_graph_pipeline→ the stage-3Tcode.model_validatedict;Tcodegains athreads: int | Nonefield (src/tablassert/lib.py).resolve_batchop in_node_opsspells the"_two"tag explicitly soself.threadscan follow positionally (compile_subgraphapplies op args positionally); from there the existingresolve_batch→lookup_rows→rs.lookup_fullmap_termsplumbing carries it into Rust unchanged.Rust fan-out beyond 16 shards
lookup_pair_terms_dbspawned exactly one reader per non-empty shard bucket.split_counts/split_bucket/plan_shard_jobs(rust/src/fullmap.rs): whenworkersexceeds the non-empty shard count, the busiest buckets are split across additional readers of the SAME shard file (greedy most-terms-per-reader, deterministic, never more readers than a bucket has terms).workers ≤ shard countis byte-identical to the previous behavior.2 × SHARD_COUNT_SHARDS.Design
default_lookup_workersandLOOKUP_PARALLEL_MINuntouched.-tc(the long-removed--table-configalias) now parses as the cluster-t cand is rejected with a coercion error instead of unknown-option; still unusable — the pinning test was updated to the new rejection mode.build_graph_pipelinecalls insrc/tablassert/agent.py) stay on auto behavior.Docs
docs/cli.md— build-kg--threadsrow (enforced by the live-surface guardrailtests/test_docs_cli_coverage.py).CHANGELOG.md— Unreleased → Added entry.Testing
make check→ exit 0:ruff checkall passed,ruff format --check+cargo fmt --checkclean,pyright0 errors.uv run pytest -q→898 passed, 35 skipped.cargo test→ 72 lib + 10 + 14 integration tests pass, incl. the newsplit_counts/split_bucketunit tests andworkers_above_shard_count_still_match_serial.cargo clippy --all-targets -- -D warnings→ clean.tablassert build-kg --helpshows the flag;tablassert build-kg x.yaml --threads 0→ exit 2 with--threads must be a positive integerbefore any build work.Questions for the reviewer
threads=Nonestill resolves toavailable_parallelism, which is no longer silently truncated at 16 — default builds on >16-core hosts now spawn more shard readers than before. Intended, or should the default stay capped at the shard count?