Skip to content

feat: add build-kg --threads to control parallel fullmap reads - #101

Merged
SkyeAv merged 1 commit into
mainfrom
increase-parallel-reads
Aug 14, 2026
Merged

feat: add build-kg --threads to control parallel fullmap reads#101
SkyeAv merged 1 commit into
mainfrom
increase-parallel-reads

Conversation

@SkyeAv

@SkyeAv SkyeAv commented Aug 14, 2026

Copy link
Copy Markdown
Owner

build-kg gains a --threads flag 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

  • Flag: build-kg accepts --threads/-t (src/tablassert/cli.py), mirroring build-fullmap --threads; a non-positive value exits 2 before any build work starts, matching the --gepa-threads gate.
  • Forwarding: threads flows build_kgbuild_pipelinebuild_graph_pipeline → the stage-3 Tcode.model_validate dict; Tcode gains a threads: int | None field (src/tablassert/lib.py).
  • Op wiring: the resolve_batch op in _node_ops spells the "_two" tag explicitly so self.threads can follow positionally (compile_subgraph applies op args positionally); from there the existing resolve_batchlookup_rowsrs.lookup_fullmap_terms plumbing carries it into Rust unchanged.

Rust fan-out beyond 16 shards

  • Why the cap wasn't redb's: redb readers hold a SHARED lock and each opens its own read transaction — any number of readers may read one shard file concurrently; only a writer conflicts. The ≤16 ceiling was an implementation choice: lookup_pair_terms_db spawned exactly one reader per non-empty shard bucket.
  • Change: new split_counts / split_bucket / plan_shard_jobs (rust/src/fullmap.rs): when workers exceeds 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 count is byte-identical to the previous behavior.
  • Determinism: hits stay tagged by input index and are re-merged by sort, so results are content- and order-identical to the serial path at any worker count — asserted by the new Rust test at 2 × SHARD_COUNT_SHARDS.

Design

  • Default unchanged in intent: unset keeps the auto behavior (large batches ≥ 1024 terms fan out, small ones stay serial); default_lookup_workers and LOOKUP_PARALLEL_MIN untouched.
  • Accepted caveat: gains are workload-dependent — largest when lookups are I/O-bound (fullmap larger than page cache on multi-hour builds), near-neutral when fully cached.
  • Accepted caveat: -tc (the long-removed --table-config alias) now parses as the cluster -t c and is rejected with a coercion error instead of unknown-option; still unusable — the pinning test was updated to the new rejection mode.
  • Deferred: the agent's measurement builds (build_graph_pipeline calls in src/tablassert/agent.py) stay on auto behavior.

Docs

  • docs/cli.md — build-kg --threads row (enforced by the live-surface guardrail tests/test_docs_cli_coverage.py).
  • CHANGELOG.md — Unreleased → Added entry.

Testing

  • make check → exit 0: ruff check all passed, ruff format --check + cargo fmt --check clean, pyright 0 errors.
  • uv run pytest -q898 passed, 35 skipped.
  • cargo test → 72 lib + 10 + 14 integration tests pass, incl. the new split_counts/split_bucket unit tests and workers_above_shard_count_still_match_serial.
  • cargo clippy --all-targets -- -D warnings → clean.
  • Manual: tablassert build-kg --help shows the flag; tablassert build-kg x.yaml --threads 0 → exit 2 with --threads must be a positive integer before any build work.

Questions for the reviewer

  • Default fan-out on big hosts. threads=None still resolves to available_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?

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown

Important

Review available on request

  • 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 52e35e7d-5bee-4c68-be99-a500d669310a


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SkyeAv
SkyeAv merged commit df70c79 into main Aug 14, 2026
5 checks passed
@SkyeAv
SkyeAv deleted the increase-parallel-reads branch August 14, 2026 19:55
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).
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