Partition and layer caching (discussion)
Superseded (2026-08-21): obikpartition and obilayeredmap are no
longer separate workspace crates — both were folded back into obikindex
as submodules (obikindex::partition, obikindex::layer), alongside the
crate's original content as obikindex::index, purely to reduce the
crate count (no behavior change). Every mention of obikpartition/
obilayeredmap as a crate below, and every dependency-direction
argument phrased in terms of "which crate depends on which" (e.g. "this
crate depends only on obilayeredmap and below, never on obikindex"),
describes that now-superseded split-crate architecture and is kept as-is
for historical context — read obikpartition::X as obikindex::
partition::X and obilayeredmap::X as obikindex::layer::X throughout.
The underlying module boundary and its rationale (Layer tier / Partition
tier / Index tier, each depending only downward) are unchanged; only the
crate-vs-module packaging changed. See obikindex::layer
for the current module doc.
Superseded, second event, same day (2026-08-21): obikpartitionner
and obikderep, the two algorithm crates, are also gone — but unlike
obikpartition/obilayeredmap above, they were not folded into
obikindex. They were first (mistakenly) merged into obikindex as an
algorithms submodule, then corrected into a new sibling crate,
obikindexer, holding obikindexer::algorithms::{partitionner,
dereplicator} and depending on obikindex — never the reverse, same
dependency direction obikpartitionner/obikderep already had. Read
obikpartitionner::X as obikindexer::algorithms::partitionner::X and
obikderep::X as obikindexer::algorithms::dereplicator::X throughout
what follows. The distinction the mistake surfaced, worth keeping: data
crates (obikindex, holding the index/partition/layer model) merge
naturally into one crate as submodules; algorithm crates that operate on
that model from outside stay separate, so the dependency only ever runs
one way.
Status (2026-08-20, latest pass): (1) done — obilayeredmap::Layer
exists, Mat is gone. (1b) done — Layer::Empty, the first non-ready
state, added (panics on every read method). (2a) done — the
obikpartition crate and KmerPartition itself exist (open/n_layers/
layer/layers/find). (2b) — migrating PartitionCache/QueryLayer
onto it — not started, deliberately deferred. (3) done — the
obikindex ↔ obikpartitionner dependency inverted: PartitionRouter now
takes &mut KmerIndex and produces Layer::Empty shells directly, closing
the gap Layer::Empty was built for in (1b) — see "(3) done" below. (4)
done — dereplication split out into its own crate, obikderep, first step
of an incremental "one algorithm at a time" split of obikpartitionner's
remaining bundle (count_kmer/build_layers not yet moved) — see "(4)
done" below. (5) — full design agreed, not yet implemented —
KmerPartition was found to be wired into nothing (KmerIndex never
calls it; every path is still computed via free functions), and the fix
turned out to be bigger than KmerPartition alone: Layer's own
constructors don't self-name either. Full redesign of both, agreed in
detail, session ended (budget) before implementation — see "(5) design
agreed" below; read it before touching KmerPartition/Layer
signatures, the shape is fully specified. (6) done — Counter, a third
algorithm, extracted from PartitionRouter the same way Dereplicator
was in (4). (7) done — LayerBuilder, the fourth and last pipeline
algorithm; the indexing pipeline is now fully decomposed into
obikindexer::algorithms::{partitionner, dereplicator, counter,
layer_builder}. (8) design agreed, item 1 done in (9) —
obikindexer::extensions::PrivateBuilder, private, six construction-only
KmerIndex methods moved out. (10) done, same session — obikindex::
IndexBuilder, public, the four maintenance methods
(clear_output_for_create/create_skeleton/finalize_indexed/state)
shared with merge/select/rebuild/reindex. Item 2 from (8)
(obikalgorithm::Algorithm) done in (12) — new crate, type Output +
fn run(&mut self) -> SKResult<Self::Output>, on_progress moved off
run()'s signature entirely into a per-algorithm .on_progress(...)
setter. Note: Layer renamed
KmerLayer (2026-08-21, outside this conversation). (5) itself still not
implemented, still first on the "order of remaining work" list. Earlier
mix-up, for
context: an earlier
version of this doc used the name KmerPartition (singular) for what was
actually the collection type (later renamed KmerPartitions, later
merged into KmerIndex — see "Major restructuring" below), and never
retracted that usage before this section was rewritten. An agent working
from that stale wording built the wrong thing. If you are about to
implement (2), read "Definitions: obikpartition and KmerPartition"
below — it is the current, authoritative naming — before touching any
other section of this file, some of which still describe superseded
states of the code and are kept only as dated history.
Definitions: obikpartition and KmerPartition (not yet created)
obikpartition — a new workspace crate, not created yet. Holds the
Partition tier of the Index { Partition { Layer } } model, the same
way obilayeredmap already holds the Layer tier as its own crate
rather than living inside obikindex. Depends only on obilayeredmap
(for Layer) and lower (obikseq, obiskio). Does not depend on
obikindex, obikpartitionner, or obikphylo. Dependency direction:
obikindex → obikpartition → obilayeredmap; obikphylo → obikindex
(and/or obikpartition directly if it ends up needing it without going
through KmerIndex).
KmerPartition (singular) — the one type this crate exists for.
Represents one partition's already-open layers — a read cache, built
once per partition and held for the run, not rebuilt per lookup. Shape:
pub struct KmerPartition {
layers: Vec<obilayeredmap::Layer>,
}
Nothing else. In particular:
- No path computation. KmerPartition::open takes an already-resolved
index_dir: &Path (plus mode: &IndexMode, n_layers: usize,
with_counts: bool — whatever it needs, as plain arguments), the same
discipline obikpartitionner::PartitionRouter::open already follows.
Computing index_dir/layer_dir from a partition number is
KmerIndex's job (obikindex, which owns that already — see "Major
restructuring" below); KmerPartition never reaches back into
KmerIndex to get it (would require obikpartition → obikindex, the
wrong direction).
- No routing/write state. Writing raw superkmers, dereplicate,
count_kmer stay in obikpartitionner::PartitionRouter — a completely
different crate, a completely different phase (pre-layer, whereas
KmerPartition only makes sense once layers exist).
- No multi-partition collection baked in. KmerPartition is one
partition. Whatever ends up caching several of them (replacing
obikphylo::siblings::cache::PartitionCache's Vec<Vec<Layer>> and
obikindex::query_layer's per-call reopen) holds Vec<KmerPartition> —
that collection can live in obikpartition too, or in obikindex
alongside KmerIndex; not yet decided, secondary to getting
KmerPartition itself right first.
Do not confuse with KmerPartitions (plural — note the s): that
type is gone. It used to be obikpartitionner's (nee obikpartition,
briefly — see the crate-rename history below, itself a separate rename
from this one) do-everything struct — routing, dereplication, and path
lookups all in one. It was deleted on 2026-08-20; its read-side (paths,
n_layers, partition_meta) was absorbed into KmerIndex, its
write-side became PartitionRouter. KmerPartition (this section,
singular, no final s) is a brand-new type with a different job, in a
crate that doesn't exist yet — not a revival, not a renaming, of
KmerPartitions.
Type-to-concept mapping: Index / Partition / Layer
The conceptual nesting Index { Partition { Layer { MPHF, Evidence, Matrix
} } } }, current state:
- Index =
obikindex::KmerIndex—{ root_path, meta: IndexMeta }. Also directly exposes the partition-path/metadata accessors (partition_dir(i),index_dir(i),layer_dir(i, l),partition_meta(i),n_layers(i),partition_mode(i),n_partitions()) sinceKmerPartitionsmerged into it (see "Major restructuring" below) —KmerIndextoday is "index + collection of partitions' paths & metadata," just without aVecof open layers. - Partition, the collection = no dedicated type today; the closest
thing is
KmerIndexitself (previous bullet). OnceKmerPartition(singular, see Definitions above) exists, aVec<KmerPartition>somewhere would be this — still open, see "Direction agreed" below. - Partition, one of them =
obikpartition::KmerPartition— to be built, see Definitions above. Nothing plays this role today;obikphylo::siblings::cache::PartitionCacheandobikindex::query_layer::QueryLayereach independently reinvent a fragment of it. - Layer =
obilayeredmap::Layer(format-erased:Count/Presence, each wrapping aTypedLayer<D>) — see "(1) done" below for how this came to be;TypedLayer<D>({ mphf: MphfLayer, data: D }, monomorphic) is the lower-level,D-fixed building blockLayeris built on, not what other crates should reach for directly. - MPHF =
MphfLayer.mphf: MemCase<MphfEps>— kmer → slot. - Evidence =
MphfLayer.ev: LayerEvidence(Exact/Approx/Hybrid—evidence.bin/fingerprint.bin; seeEvidenceKind). - Matrix =
TypedLayer<D>.data: D—PersistentBitMatrix/PersistentCompactIntMatrix.
Target nesting once KmerPartition exists:
KmerIndex (obikindex)
└─ (opened on demand, per i) KmerPartition (obikpartition — not yet built)
└─ layers: Vec<Layer> (obilayeredmap)
└─ Layer::Count/Presence(TypedLayer<D>)
└─ TypedLayer<D> { mphf: MphfLayer, data: D }
├─ mphf.mphf → MPHF
├─ mphf.ev → Evidence
└─ data → Matrix
Major restructuring (2026-08-20): KmerPartitions merged into KmerIndex
Prompted by a direct question: why keep KmerIndex/KmerPartitions split
when, one level down, KmerPartitions is going to directly hold
Vec<KmerPartition> rather than being split again into
"collection-holder" + "collection"? Investigating the actual justification
("KmerPartitions has an independent lifecycle, used before an index
exists") turned out to be false — KmerPartitions::create was called
in exactly one place, inside KmerIndex::create, and every
open_with_config reopen outside KmerIndex's own constructors was a
redundant re-derivation of a KmerPartitions already reachable via
index.partition() (the exact kind of duplication this whole doc has been
tracking). Once that was gone, so was the reason to keep them separate.
Second correction, from the same conversation: obikpartitionner had
accumulated query/merge/select/rebuild/dump/distance logic that has
nothing to do with partitioning super-kmers — it operates on layers,
which don't exist yet at the phase obikpartitionner is actually
responsible for (scatter → dereplicate → count, all pre-layer). That
logic moved to obikindex, which already depends on obilayeredmap and
never needed obikpartitionner for it. No crate-dependency inversion was
needed — obikindex → obikpartitionner stays the same direction as before.
Result:
- obikpartitionner (renamed back from obikpartition) now contains only
PartitionRouter (superkmer routing: write/write_batch/flush/
close, dereplicate, count_kmer, KmerSpectrum) and the
partition_dir(root, i) naming primitive both PartitionRouter and
KmerIndex build on. KmerPartitions no longer exists as a type.
- KmerIndex (obikindex) absorbed KmerPartitions's read-side entirely:
partition_dir/index_dir/layer_dir/partition_meta/n_layers/
partition_mode/n_partitions (the last now derived from
2^config.n_bits, no longer a stored, independently-set duplicate field
— kmer_size/minimizer_size used to be double-stored, in both
KmerPartitions and IndexMeta.config, a latent-drift risk flagged
earlier in this doc; now single-sourced from IndexMeta.config). Seven
whole files moved from obikpartitionner into obikindex verbatim as
impl KmerIndex blocks, kept as separate files (not merged into
existing same-topic files): index_layer.rs, query_layer.rs,
merge_layer/, select_layer.rs, rebuild_layer.rs, dump_layer.rs,
plus distance.rs's count_store/presence_store (renamed
matrix_store.rs to avoid colliding with obikindex's own pre-existing
distance.rs), and their shared support (common.rs's load_meta/
olm_to_sk, filter.rs, graph_pipeline.rs).
- obikphylo::siblings::cache::PartitionCache::build now takes &KmerIndex
directly instead of a separately-opened &KmerPartitions — this deleted
the redundant-reopen pattern at all 8 call sites
(alignment/build/cardinality/distance/entropy×2/
sankoff_bundle/stats), the same bug flagged earlier in this
conversation as a side effect of investigating the false "independent
lifecycle" claim.
- KmerIndex::partition()/partition_mut() are gone; scatter()
(obikmer) and any write-side code get a transient PartitionRouter via
KmerIndex::partition_router().
- A real bug caught by the test suite during this move:
PartitionRouter::open initially defaulted to closed: true (inherited
from KmerPartitions::open_with_config's old read-only-reopen
semantics), which broke every write through a router obtained via
partition_router(). Fixed — PartitionRouter is exclusively a
write/processing tool now, so open always starts open.
Full workspace test suite green (0 failed) after, including all 27
obikphylo::siblings tests.
(1) done (2026-08-20): Layer is now the heterogeneous handle, Mat is gone
Resolved the naming question left open above. Layer<D> (the old
generic/monomorphic type) renamed to TypedLayer<D> throughout
(obilayeredmap, obikindex, obikphylo — 12 files, mechanical) to free
Layer for the type that's actually meant to be everyone's default
handle. obilayeredmap::content_layer::Layer (re-exported at the crate
root) is that type — Count(TypedLayer<PersistentCompactIntMatrix>)/
Presence(TypedLayer<PersistentBitMatrix>), Layer::open doing the same
disk probe Mat::open used to, find_slot/index_batch/n_cols/
fill_sub_matrix_carries dispatching per variant exactly as Mat did.
obikphylo::siblings::cache::Mat deleted outright — PartitionCache now
holds Vec<Vec<obilayeredmap::Layer>> directly. The one sibling-specific
method Mat carried (iter_minorants_batch) is not on obilayeredmap::
Layer (phylo concepts don't belong in obilayeredmap) — it's an
impl SiblingLayerExt for obilayeredmap::Layer in iter.rs, dispatching
to each variant's existing impl<D: LayerData> SiblingLayerExt for
TypedLayer<D>.
Full workspace suite green (0 failed) after, including all 27
obikphylo::siblings tests.
Still not built: (2) — KmerPartition (singular, one partition's open
Vec<Layer>) and a multi-partition cache in obikpartitionner to replace
obikphylo::siblings::cache::PartitionCache and obikindex::query_layer's
still-separate QueryLayer (which still independently bundles MPHF+matrix,
2-way not using Layer at all). Both remaining consumers now sit one
Layer::open call away from unifying onto (2) once it exists.
(1b) done (2026-08-20): Layer::Empty — the first non-ready-to-read state
First step toward Layer representing a layer's whole life, not just the
open-for-reading end of it (see "Definitions" above: KmerPartition will
hold Vec<Layer> regardless of each layer's state, states in between
included). Added one variant:
pub enum Layer {
Empty { dir: PathBuf },
Count(TypedLayer<PersistentCompactIntMatrix>),
Presence(TypedLayer<PersistentBitMatrix>),
}
Layer::create(dir) makes the directory and returns Empty { dir } —
nothing else; no MPHF/unitigs/evidence construction yet (that's the
deferred next step: build_mphf()/build_unitigs()/build_evidence()
methods to progress Empty → eventually Count/Presence). Empty
carries path accessors so builder code has one place to get
mphf_path()/unitigs_path()/evidence_path()/fingerprint_path()/
counts_dir()/presence_dir() from, instead of redeclaring the
mphf.bin/unitigs.bin/… filenames at each write site — reusing the
constants layer.rs/mphf_layer.rs already own (COUNTS_DIR/
PRESENCE_DIR widened from private to pub(crate), file-name constants
already were).
Every read method (content/evidence_kind/n/find_slot/
index_batch/n_cols/fill_sub_matrix_carries) panics on Empty with a
one-line message naming the method — confirmed as the right behaviour:
calling any of them on an Empty layer means the caller assumed a layer
was ready when it wasn't, an implementation error to surface loudly, not
a case to design around (Option/Result would let it silently
propagate instead of failing at the actual mistake). Same panic added to
obikphylo::siblings::iter.rs's impl SiblingLayerExt for
obilayeredmap::Layer (4 methods), the one other place that exhaustively
matched Layer's variants.
Full workspace suite green (cargo check --workspace --all-targets then
cargo test --workspace, exit code 0) after.
Still deferred, per explicit instruction: build_mphf()/
build_unitigs()/build_evidence() to progress Empty further, and (2)
— KmerPartition itself — unchanged from above (see "(2a) done" below,
added next).
(2a) done (2026-08-20): obikpartition crate + KmerPartition
Built exactly the shape "Definitions" (top of file) specifies, nothing
more — deliberately scoped down from the full "Direction agreed" plan
below: only steps 1–2 (open/n_layers/layer/layers/find), not 3–4
(migrating PartitionCache/QueryLayer onto it), per explicit
instruction to implement KmerPartition first and decide the wiring
("comment on branche tout ça dans la construction") separately, later.
pub struct KmerPartition {
layers: Vec<obilayeredmap::Layer>,
}
impl KmerPartition {
pub fn open(index_dir: &Path, mode: &IndexMode, n_layers: usize, with_counts: bool) -> OLMResult<Self>;
pub fn n_layers(&self) -> usize;
pub fn layer(&self, i: usize) -> &Layer;
pub fn layers(&self) -> &[Layer];
pub fn find(&self, kmer: CanonicalKmer) -> Option<usize>;
}
open takes index_dir/mode/n_layers/with_counts as plain
arguments — no reach-back into KmerIndex (would need obikpartition →
obikindex, the wrong direction) — and builds each layer's path via
obilayeredmap::layer_dir(index_dir, l), the same shared naming
primitive KmerIndex::layer_dir itself delegates to, not a second copy of
the layer_N convention. find mirrors PartitionCache::find's
semantics (first layer that carries the kmer wins) but doesn't yet cover
find_presence_batch/find_presence_batch_fast — those exist only to
serve PartitionCache, so they're part of the (2b) migration, not this
step; building them now against the current sibling-specific tuple shape
(CanonicalKmer, usize, u8, u8) would either bake phylo vocabulary
(family_idx, base) into obikpartition or require deciding a generic
payload shape — a real design fork, deferred to when (2b) is actually
tackled rather than guessed at here.
Crate deps: obikseq, obilayeredmap only (dev-deps add obiskio,
obicompactvec, tempfile for tests) — matches the "Definitions"
constraint (obikpartition depends on obilayeredmap and below, never
obikindex/obikpartitionner/obikphylo). Registered as a new workspace
member (src/Cargo.toml). 3 new tests (open_reads_every_layer_in_order,
find_reports_the_first_layer_that_carries_the_kmer,
find_returns_none_for_an_absent_kmer). Full workspace suite green
(cargo check --workspace --all-targets then cargo test --workspace)
after.
Still not done: (2b) — migrating obikphylo::siblings::cache::
PartitionCache (currently Vec<Vec<Layer>>) and
obikindex::query_layer::QueryLayer (currently uncached, bypasses Layer
entirely) onto KmerPartition/Vec<KmerPartition>; deciding whether that
collection lives in obikpartition or obikindex; deciding the
batch-lookup surface's exact shape (generic payload vs. as-is sibling
tuple moved in wholesale); scan_layer_families's still-independent
PartitionMeta::load (see "Remaining instance…" below) — all explicitly
deferred to whenever wiring is tackled next.
(3) done (2026-08-20): obikindex ↔ obikpartitionner dependency inverted, PartitionRouter now fills Layer::Empty shells
Resolved a question left implicit since "Major restructuring": that pass
set the direction obikindex → obikpartitionner (so KmerIndex could
delegate partition_dir to it) without questioning whether that was the
right direction at all. Challenged directly: obikpartitionner is an
algorithm (superkmer routing/dereplication/counting) operating on an
index (KmerIndex, the data structure) — algorithms depend on the data
types they need, not the other way around. [[feedback_no_precedent_defense]]
applied here: "that's the direction we already picked" was not treated as
a justification for keeping it.
New direction: obikpartitionner → obikindex (+ obilayeredmap,
obipipeline, obiread directly, for what run's pipeline itself needs).
obikindex → obikpartitionner is gone entirely — KmerIndex no longer
imports PartitionRouter/KmerSpectrum in any form. Two path-naming
primitives that used to make this edge necessary moved down a tier instead
of staying put:
- partition_dir/PARTITIONS_SUBDIR moved from obikpartitionner into
obikpartition (the Partition-tier crate KmerPartition already lives
in), alongside a new index_dir(root, i) — both free functions,
mirroring obilayeredmap::layer_dir one tier down. KmerIndex::
partition_dir/index_dir now delegate here instead of to
obikpartitionner/an inline .join("index").
- KmerIndex::create/create_skeleton no longer call
PartitionRouter::create to lay out an empty partitions/ skeleton
upfront — turned out to be dead weight once traced: select_layer.rs/
rebuild_layer.rs already create_dir_all their own partition/layer
directories on demand, and Layer::create's directory-creation covers
the scatter path the same way. Partitions and their layer-0 shells now
come into existence lazily, on first write, with nothing to pre-create.
KmerIndex::create's now-unused force: bool parameter was dropped
(4 call sites updated) rather than left as a dead parameter.
PartitionRouter reshaped (obikpartitionner/src/partition/router.rs)
around the "création, paramétrage, run()" shape agreed on: new(index:
&mut KmerIndex) -> Self (no disk access), chainable setters
(level_max/theta/workers/max_open, defaults matching the CLI's old
hardcoded values), then run(path_source, on_progress). write/
write_batch/flush/close/dereplicate/count_kmer stay public,
unconsumed (&self/&mut self, not self) — callers needing fine-grained
control (tests, obikphylo's test harness) still get it, run is a
convenience layered on top, not the only way in.
run absorbs the entire body of what used to be the free function
obikmer::steps::scatter (now deleted, along with the steps module
entirely) — the obipipeline::make_pipe! two-stage pipeline
(file→pages→superkmers), throttling, per-file logging. What changed:
- Every ensure_writer(partition) call now does Layer::create(&layer0_dir)
(layer0_dir = obilayeredmap::layer_dir(&index.index_dir(i), 0)) before
opening raw.{ext} inside it — raw/dereplicated superkmer files and the
provisional mphf1.bin/counts1.bin/kmer_spectrum_raw.json now live
under <partition>/index/layer_0/, not flat under <partition>/ as
before. This is Layer::Empty actually being used as the "builder code
holding an Empty layer" its own (1b) docs anticipated, not just a shell
with no consumer.
- Caught by an end-to-end smoke test, not by cargo test: this path
move broke obikindex::index_layer::build_index_layer and
remove_build_artifacts, both of which still read/deleted
dereplicated.skmer.zst/mphf1.bin/counts1.bin from
self.partition_dir(i) (the old flat location) — no test in the
workspace suite exercises the real CLI's file-reading scatter path
end-to-end (obikphylo's test harness and obikpartitionner's own
tests both call write_batch directly, bypassing run/file discovery
entirely), so the whole suite stayed green while obikmer index on
real FASTA silently indexed 0 kmers. Found by running the actual CLI
against a small FASTA and noticing count.json's f0 (870, correct)
didn't match "0 total kmers indexed" at the final stage. Fixed by
retargeting both functions to self.layer_dir(i, 0). Lesson,
consistent with the retracted-claim lesson above: a green test suite
is not proof a refactor is correct when no test in it exercises the
specific path that changed — for anything touching the CLI's own
file-driven entry point, running the CLI for real is not optional
verification.
- The internal obisys::spinner("scatter") + hand-rolled EMA-rate display
is gone from the library entirely, replaced by an Option<impl
FnMut(obisys::Progress)> parameter — a new, deliberately generic
progress-reporting type (obisys::Progress { position: u64, total:
Option<u64> }, alongside the existing TracedBar/spinner/
progress_bar) added specifically so every future algo crate's run()
reports progress the same shape, once, rather than each inventing its
own. total: None here (bases processed isn't knowable without
pre-scanning every input file) — deliberately simpler than the old
in-library rate/file-count/thread-count message; the caller can
recompute a Mbp/s rate from consecutive position values +
wall-clock time itself, which is exactly what cmd/index/mod.rs now
does to reproduce the old spinner message. This is a real, intentional
restriction of the library's job: it reports raw ticks, the CLI decides
what a human sees — same "generic vs. domain-specific" split applied
again, this time to progress reporting rather than to Layer content.
Explicitly not the same mechanism as Stage/Reporter (per
[[feedback_stage_reporter_in_cmd_layer]]): Stage/Reporter measures a
whole call's wall time from outside it; a progress callback has to fire
from inside a loop mid-call, which wrapping from outside cannot
express — two different needs, not the same rule reapplied under a new
name. Stage::start("scatter")/rep.push(...) stayed in
cmd/index/mod.rs, wrapping the whole run() call, unchanged in kind.
- dereplicate/count_kmer keep their existing internal
obisys::progress_bar(...) calls as-is (unconverted to the callback) —
explicitly out of scope for this pass, by agreement.
Forced, not optional, consequence of the dependency inversion:
KmerIndex::dereplicate_and_count/partition_router/write_spectrum(&
KmerSpectrum) could not stay on KmerIndex at all once obikindex can no
longer name obikpartitionner::{PartitionRouter, KmerSpectrum} in any
position — not a design choice, a mechanical requirement of severing the
edge. Replaced by: KmerIndex::write_spectrum(f0: u64, f1: u64, counts:
&BTreeMap<u32, u64>) (plain values, no KmerSpectrum dependency) and a
new KmerIndex::mark_counted() (symmetric to the already-existing
mark_scattered), with the orchestration itself (router.dereplicate() →
router.count_kmer() → write_spectrum → mark_counted()) now living in
cmd/index/mod.rs, not obikindex.
Every PartitionRouter::new(&mut index) call in this codebase runs into
the same NLL trap once: PartitionRouter has a Drop impl (auto-close
on scope exit), which extends its &mut KmerIndex borrow to the end of
the enclosing scope even after its last real use — idx.mark_scattered()
right after router.run(...) (or idx.write_spectrum(...) right after
router.count_kmer(...)) fails to borrow-check unless the router is
drop()-ed explicitly first. Hit and fixed identically at all three call
sites that needed it (cmd/index/mod.rs ×2, obikphylo's test harness,
obikpartitionner's own tests).
Full workspace suite green (cargo check --workspace --all-targets +
cargo test --workspace, exit code 0) both before and after the
index_layer.rs fix above — the smoke test is what actually caught the
regression the suite missed.
(4) done (2026-08-20): obikderep — dereplication split out of obikpartitionner, one algorithm at a time
Follow-on question after (3): the indexing pipeline has 4 stages (scatter,
dereplicate, count_kmer, index-build — see the CLI's own Reporter output,
one line per stage), but obikpartitionner — a name that says
partitioning — owned three of them (routing, dereplication, counting).
Challenged directly, same as (3)'s dependency-direction question: a crate
should hold what its name says, not accumulate unrelated stages just
because they happened to land there first. Two ways to fix it — one crate
renamed to hold all remaining stages, or one crate per stage — decided in
favour of the latter, explicitly incremental: build the second algo
crate first (obikderep, dereplication only), only then look at what it
and PartitionRouter actually have in common, and factor a shared
Algorithm trait (future obikalgorithm crate) from that real overlap —
not guessed at from a single example. count_kmer and build_layers
(currently KmerIndex inherent methods — itself flagged as inconsistent
with "KmerIndex is a data structure, not a compute structure") are left
alone this round, on purpose — one stage moves at a time.
obikderep (new crate): Dereplicator<'a> { index: &'a KmerIndex, n_partitions, level }
— new(index: &KmerIndex) (shared borrow, not &mut: dereplication never
writes index metadata), no setters yet (nothing to configure), run(on_progress)
does the two-phase split+merge dereplication in parallel across partitions,
ported unchanged from PartitionRouter::dereplicate (moved wholesale:
optimal_buckets/dereplicate_partition/load_bucket/flush_map/
remove_skmer_file, now private to this crate in dereplicate.rs).
obikpartitionner::PartitionRouter::dereplicate is gone; count_kmer
stays.
A real signature difference from PartitionRouter::run, not an
inconsistency: Dereplicator::run takes Option<impl Fn(Progress) +
Sync>, not FnMut. PartitionRouter::run's callback is invoked from one
sequential loop (FnMut is fine); Dereplicator::run's work is
rayon::par_iter, so the callback can be invoked concurrently from
multiple worker threads — same reason obisys::TracedBar's own methods
take &self, not &mut self. Progress position is tracked with an
AtomicU64, incremented from inside the parallel closure so each
completed partition reports immediately — collecting all results first and
reporting after (the first draft of this) would have delivered every tick
in one burst at the very end, defeating the point of a live progress bar.
total: Some(n_partitions) (known up front, unlike scatter's bases count)
— cmd/index/mod.rs renders a real progress_bar, not a spinner, driven
by the callback exactly like scatter's spinner is.
A second, pre-existing instance of the exact bug (3) fixed, caught
before it shipped: dereplicated.skmer.zst was hand-built as a string
literal independently in three places — obikpartitionner's
dereplicate.rs/count.rs and obikindex's index_layer.rs (a literal
that already predated this session, never caught until now). Splitting
dereplication into its own crate turns this from "two places, still
matching by luck" into "three independent crates that must agree on a
filename with no shared dependency forcing them to" — no longer
deferrable. Fixed by adding obilayeredmap::{raw_superkmers_path,
dereplicated_superkmers_path} (free functions, layer_dir: &Path ->
PathBuf, mirroring layer_dir itself) — the filename lives in one place,
in the Layer-tier crate every consumer here already depends on
(obikpartitionner, obikderep, obikindex all reach it without a new
edge), and no external crate ever sees the literal "skmer.zst" again.
This reverses (3)'s own earlier call to keep SK_EXT private to
obikpartitionner — that call assumed a single owner; a second owner
appearing (obikderep) removed the assumption it rested on, so the
decision changed with it, not out of inconsistency.
Every count_kmer call site that used to run after router.dereplicate()
on the same PartitionRouter now runs after a separate
Dereplicator::new(&idx).run(...) call, on a freshly-constructed
PartitionRouter — PartitionRouter no longer offers a combined
"dereplicate then count" path. Updated at all three call sites that had
one: cmd/index/mod.rs, obikphylo's test harness, obikpartitionner's
own tests.
Full workspace suite green (cargo check --workspace --all-targets +
cargo test --workspace, exit code 0), plus an end-to-end CLI smoke test
against real FASTA data (scatter → dereplicate → count → index-build →
query, same numbers as (3)'s smoke test: 870 kmers) — required this time
too, per (3)'s own lesson: no test in the suite exercises obikmer index's
real file-driven path.
Still not done: count_kmer/build_layers staying where they are, the
obikalgorithm shared-trait extraction (deliberately deferred until a
third data point exists), and everything already listed under (2b).
(5) design agreed, not yet implemented (2026-08-20): KmerPartition rewritten, Layer gains self-naming, a future cache crate over KmerIndex
Session ended (out of budget) before any of this was coded. Everything below is a fully specified plan, agreed sentence by sentence with the user — not a sketch to re-derive, not a proposal to re-litigate. Implement it as written; if something here turns out to be wrong once coded, fix it and update this section, don't restart the design conversation.
How this was found
Direct question from the user: "tu as bien créé une structure
KmerPartition ?" — yes (2a), but investigating exposed that it is
wired into nothing. KmerIndex has no partition(i) method at all;
partition_dir/index_dir/layer_dir still call obikpartition::
partition_dir/index_dir and obilayeredmap::layer_dir as bare free
functions directly, never touching a KmerPartition/Layer object to get
there. The end result on disk is identical (same paths), which is exactly
why no test caught it — but the responsibility is in the wrong place:
one function (on KmerIndex) knows the whole three-tier naming
convention, instead of each tier asking the one below it for its own
path. User's framing, verbatim, now saved as [[feedback_no_spaghetti_petits_pois]]:
"spaghetti" (logic untraceable, split across too many unrelated crates)
and "petits pois" (small bits of naming logic dispersed with no owning
object) are strictly forbidden — this was a live example of both.
Pushed further, twice:
1. First correction: Layer::create(&obilayeredmap::layer_dir(&dir, 0)) —
still a free-function call from outside Layer to compute where it
should live. "Le layer n'est pas con, c'est lui qui dit où est-ce qu'il
doit être sauvé" (the layer isn't stupid, it says itself where it
should be saved).
2. Second correction, the general principle: "une partition est juste
identifiée par un numéro, tout se calcule à partir du numéro, et un
layer est identifié à partir d'un numéro et tout se calcule à partir de
ce numéro." Concretely: each object stores its own local identifying
number plus its immediate parent's path (captured once, at
construction) — never a path handed in again later by a caller, and
never a free function outside the object that can compute that path
independently. Explicitly rejected along the way: making users pass
"the partition's path that contains the layer" to open a layer — the
parent path is captured once, at the child's construction, not
re-supplied at every call.
The agreed shape
Layer (obilayeredmap) — identified by l + its parent partition's
directory, both captured at construction, never received again:
pub enum Layer {
Empty { partition_dir: PathBuf, l: usize }, // pure identification, no disk I/O
Count(TypedLayer<PersistentCompactIntMatrix>),
Presence(TypedLayer<PersistentBitMatrix>),
}
impl Layer {
pub fn at(partition_dir: &Path, l: usize) -> Self; // identify only
fn dir(&self) -> PathBuf; // private — layer_dir() no longer a public free function, folded in here
pub fn create(self) -> io::Result<Self>; // creates the directory if needed; no path parameter anymore
pub fn open(self, mode: &IndexMode, with_counts: bool) -> OLMResult<Self>; // no path parameter anymore
// mphf_path()/unitigs_path()/evidence_path()/fingerprint_path()/counts_dir()/presence_dir()
// unchanged in spirit, implemented via self.dir() instead of a stored `dir` field read directly
}
Note this replaces Layer::Empty { dir: PathBuf } from (1b) — dir
becomes a computed value (partition_dir.join(format!("layer_{l}"))), not
a stored field. obilayeredmap::layer_dir/raw_superkmers_path/
dereplicated_superkmers_path (currently public free functions,
introduced in (3)/(4)) stop being called from outside obilayeredmap
entirely once this lands — they were the right fix for their moment (a
second crate, obikderep, needed to agree on a filename with no owner),
but the real fix, now visible with a third data point, is that Layer
itself should be the only thing anyone asks.
KmerPartition (obikpartition) — same principle, one tier up:
pub struct KmerPartition {
index_root: PathBuf, // the parent KmerIndex's root, captured once
i: usize,
}
impl KmerPartition {
pub fn new(index_root: PathBuf, i: usize) -> Self; // identify only, no disk I/O
pub fn create(index_root: PathBuf, i: usize) -> io::Result<Self>; // creates this partition's directory + an empty layer 0 (a partition is never born without one — that knowledge lives here, not in whoever calls create)
pub fn partition_dir(&self) -> PathBuf; // part_{i:05}
pub fn index_dir(&self) -> PathBuf; // part_{i:05}/index
pub fn layer(&self, l: usize) -> Layer; // Layer::at(&self.index_dir(), l) — caller never touches a path
pub fn meta(&self) -> SKResult<PartitionMeta>; // n_layers + mode; must absorb the recovery-on-missing-file logic
// currently private in obikindex::common::load_meta (obikpartition
// can't depend on obikindex to reuse it — this logic moves down)
pub fn n_layers(&self) -> SKResult<usize>; // meta()?.n_layers
pub fn mode(&self) -> SKResult<IndexMode>; // meta()?.mode — "exact/approximatif"
pub fn is_filled(&self) -> bool; // does this partition's directory exist at all
pub fn n_kmers(&self) -> io::Result<usize>; // LayerMeta::load(&self.layer(0).dir()).n — reads layer 0's count as
// a representative figure, same "read the first one" trick
// n_layers_per_partition() already uses at the KmerIndex level
}
This replaces (2a)'s KmerPartition { layers: Vec<Layer> } entirely
— no eagerly-opened Vec<Layer>, no find() (both belong to the future
cache, see below, which is the thing that actually holds opened layers
alive across many lookups). (2a)'s version is safe to delete outright: it
was never wired into anything (confirmed above), so nothing depends on
its current shape. New dependencies needed: obikpartition gains
obiskio (for SKResult) and obicompactvec (for LayerMeta).
Deliberately not built this round: cross-level consistency checks ("verify everything below me is in the same state") — a real idea, raised by the user, but nothing concrete needs it yet; building it speculatively would be exactly the premature-abstraction pattern this project avoids.
KmerIndex (obikindex) — becomes the sole entry point:
pub fn partition(&self, i: usize) -> KmerPartition; // KmerPartition::new(self.root_path.clone(), i)
partition_dir(i)/index_dir(i)/layer_dir(i, l) stay as public
methods (≈30 existing call sites across obikindex/obikphylo — see (3)'s
option A, applied identically here) but become pure delegations:
self.partition(i).partition_dir(), self.partition(i).index_dir(),
self.partition(i).layer(l).dir() (needs Layer::dir() to be visible
enough for this — likely pub(crate) in obilayeredmap plus a thin
public wrapper, or a public accessor on Layer itself; not fully nailed
down, decide while implementing). No caller outside obikindex changes.
Known blast radius (why this wasn't done in the same session)
- 14 files call
Layer::open/Layer::createdirectly today (obikphylo/siblings/{cache,build,family_scan,tests}.rs,obikpartitionner/partition/router.rs,obikpartition/src/lib.rs,obikindex/{rebuild_layer,dump_layer,index,query_layer}.rs,obilayeredmap/{mphf_layer,layer,map,content_layer}.rs) — every one loses its path parameter and gains a(partition_dir, l)or an already-identifiedLayerto call.create()/.open()on instead. - ≈30 files call
KmerIndex::partition_dir/index_dir/layer_dir— unaffected in their own code (same public signatures), but worth re-checking once (5) lands that none of them were relying on the old free-function-based implementation in a way the new delegation breaks. obikpartitionner::PartitionRouter::ensure_writerandobikderep'srunboth currently callobilayeredmap::{layer_dir, raw_superkmers_path, dereplicated_superkmers_path}directly (from (3)/(4)) — both need to switch to going throughindex.partition(i).layer(0)instead.
Also agreed, separately: PartitionRouter::new never needed &mut KmerIndex
Verified by reading the code: every call PartitionRouter makes on
index is &self (index.kmer_size(), index.index_dir(i)). The &mut
in its current signature (from (3)) was inherited from the original
"the router writes to the partitions" reasoning, never actually required
by any method call. This is exactly what caused every drop(router)
workaround needed throughout (3)/(4) (cmd/index/mod.rs ×2, obikphylo's
test harness, obikpartitionner's own tests) — PartitionRouter holds a
Drop impl, which extends a &mut borrow to the end of its scope even
past its last real use. Fix alongside (5): change
PartitionRouter::new(index: &'a mut KmerIndex) to &'a KmerIndex, and
remove the now-unnecessary drop(router) calls at all four sites.
Also discussed: a future cache crate, not part of (5), not obikalgorithm either
Separate idea, explicitly not part of this design and not started:
a new crate whose only job is to cache open KmerPartitions (and their
opened Layers) across one run — replacing both obikphylo::siblings::
cache::PartitionCache (today, sibling-specific, holds Vec<Vec<Layer>>)
and obikindex::query_layer::QueryLayer (today, uncached, bypasses
Layer entirely) — the two consumers (2b) already identified as each
reinventing a fragment of the same thing.
User's framing: this is not a third obikalgorithm data point — an
algorithm has a new → run → done shape; a cache has a fundamentally
different one (open, stay alive for a whole run, serve lookups, maybe
evict) — "on crée un cache sur un index, ça consomme un index." Two
distinct crate roles in this ecosystem (data crates: obikpartition/
obilayeredmap; algorithm crates: obikpartitionner/obikderep/future
obikalgorithm implementors; and now a cache/service crate), not one
unified shape to force everything into.
Depends on (5) being done first: the cache crate's whole job is holding
Vec<KmerPartition>/opened Layers alive, built via KmerIndex::
partition(i) as its factory — nothing to build it on top of until (5)
lands. Still open once (5) is done: eviction policy vs. holding everything
open for the process lifetime (the never-measured mmap/VM-mapping-count
question from earlier in this doc), and whether it lives in obikpartition
itself or a new crate.
Order of remaining work, as currently understood
- (5) —
Layer/KmerPartition/KmerIndexrewrite described above, plus thePartitionRouter&mut→&fix (same root cause, same session, do together). - The future cache crate (name not chosen), consuming
KmerIndex:: partition(i)— unblocks migratingPartitionCache/QueryLayer(2b). obikalgorithm— still deliberately waiting for a thirdrun()-shaped data point (count_kmerorbuild_layersmigrating out ofKmerIndex/PartitionRouter) before extracting a shared trait; two examples were judged not enough to be sure of the shape (Fn+SyncvsFnMutcallback bound already diverged between the two that exist).
(6) done (2026-08-21): Counter — third algorithm, extracted the same way as Dereplicator
Between (5) and this, the user did a session of their own crate
restructuring (see the two "Superseded" notes at the top of this file):
obikpartition/obilayeredmap folded into obikindex as submodules
(obikindex::partition, obikindex::layer), and obikpartitionner/
obikderep merged into one sibling crate, obikindexer, holding
obikindexer::algorithms::{partitionner, dereplicator}. (5)'s design
(Layer/KmerPartition self-naming by number, PartitionRouter's
&mut → & fix) was not part of that — pure crate/module packaging,
confirmed by reading the actual code (Layer::open/create still take an
external dir: &Path, KmerPartition still eagerly opens all layers,
PartitionRouter still holds &mut KmerIndex). (5) remains exactly as
specified, not yet implemented.
This step: count_kmer (still living on PartitionRouter, per (4)'s own
"still not done" note) extracted into obikindexer::algorithms::counter::
Counter, mirroring Dereplicator exactly — third data point for the
eventual obikalgorithm trait, still not extracted (still only 3 examples
with 2 different callback bounds; holding off per (5)'s "order of
remaining work").
pub struct Counter<'a> {
index: &'a KmerIndex,
n_partitions: usize,
keep_partial: bool,
}
impl<'a> Counter<'a> {
pub fn new(index: &'a KmerIndex) -> Self;
pub fn keep_partial(mut self, v: bool) -> Self; // setter, mirrors PartitionRouter's style; defaults to false
pub fn run(&self, on_progress: Option<impl Fn(Progress) + Sync>) -> SKResult<KmerSpectrum>;
}
Same shape as Dereplicator throughout: Fn(Progress) + Sync (not
FnMut) since counting is also a parallel par_iter over partitions, an
AtomicU64 position counter incremented from inside the parallel closure
so progress reports arrive in real time rather than bursting at the end
once .collect() finishes, total: Some(n_partitions) (known up front).
KmerSpectrum (the {f0, f1, counts} aggregate) moved from
partitionner::router to counter, since it's Counter::run's return
value now, not PartitionRouter's. count.rs/kmer_sort.rs moved
verbatim from partitionner/ to counter/ (unchanged bodies — only
count_kmer itself, KmerSpectrum, and the imports they pulled in were
removed from router.rs).
One divergence from Dereplicator: a keep_partial setter exists (no
equivalent on Dereplicator, which has no setters at all) — a real,
already-present parameter (keep_intermediate at the CLI), not a
speculative addition.
count_kmer's three former callers (obikmer::cmd::index, obikphylo's
test harness, obikindexer::algorithms::partitionner's own
pipeline_counts test helper) all updated to Counter::new(&idx).
run(...) — the last one simplified further: it used to read back
kmer_spectrum_raw.json from disk after calling count_partition
directly (white-box), now it just uses the KmerSpectrum Counter::run
already returns.
Full workspace suite green (cargo check --workspace --all-targets +
cargo test --workspace, exit code 0), plus an end-to-end CLI smoke test
against real FASTA data (scatter → dereplicate → count → index-build →
query) — required every time per (3)'s lesson, and it earned its keep
again: the very first smoke-test query returned zero matches, which
looked like a regression until traced to the query sequence itself being
low-complexity ("GGCCCCCCACG", six same-base runs) and rejected by
query's own default entropy threshold — nothing to do with this change.
Re-tested with a different substring, confirmed working (kmer found,
count matched the index).
Still not done: (5) (Layer/KmerPartition redesign, PartitionRouter's
&mut→&), the future cache crate, build_layers (still a KmerIndex
inherent method, not an algorithm), and obikalgorithm itself.
(7) done (2026-08-21): LayerBuilder — fourth and last pipeline algorithm
Closes out the indexing pipeline: build_layers/build_index_layer
(the last stage still living as KmerIndex inherent methods, flagged as
inconsistent since (6)) extracted into obikindexer::algorithms::
layer_builder::LayerBuilder, same two-phase shape as the other three.
pub struct LayerBuilder<'a> {
index: &'a KmerIndex,
n_partitions: usize,
min_abundance: u32,
max_abundance: Option<u32>,
keep_intermediate: bool,
}
impl<'a> LayerBuilder<'a> {
pub fn new(index: &'a KmerIndex) -> Self;
pub fn min_abundance(mut self, v: u32) -> Self;
pub fn max_abundance(mut self, v: Option<u32>) -> Self;
pub fn keep_intermediate(mut self, v: bool) -> Self;
pub fn run(&self, on_progress: Option<impl FnMut(Progress) + Send>) -> SKResult<usize>; // returns total kmers built
}
Different from all three prior extractions in one respect, deliberately
not "fixed" to match them: the actual per-partition construction logic
(De Bruijn graph from dereplicated superkmers + provisional counts →
unitigs → MPHF → matrix) stayed put as KmerIndex::build_index_layer/
remove_build_artifacts (both already pub) — not moved into
obikindexer. Checked first: unlike dereplicate_partition/
count_partition (which only ever had one caller), build_index_layer
depends on several obikindex-internal helpers (graph_pipeline::
{write_graph_as_unitigs, materialize_layer}, common::olm_to_sk) that
are pub(crate) and shared with merge/select/rebuild's own
layer-construction paths — moving build_index_layer out would have
meant either exporting that internal surface just for this one algorithm
or duplicating it. Neither was needed: build_index_layer/
remove_build_artifacts were already public KmerIndex methods, so
LayerBuilder's job is purely the orchestration around them (scheduling,
config, progress) — the exact same "algorithm calls already-public
KmerIndex primitives" shape PartitionRouter/Dereplicator/Counter
already have, just at a coarser grain for this one stage. This is the
"is the producer's API actually deficient?" check from
[[feedback_no_spaghetti_petits_pois]] applied and answered "no" — not
skipped.
Two more real divergences, both forced by PartitionRunner, not
arbitrary:
- Uses obikindex::PartitionRunner (NUMA-aware scheduler, already
pub used from obikindex) instead of plain rayon::into_par_iter
like Dereplicator/Counter — matches what build_layers already used
before extraction; this stage is more CPU/memory-intensive per partition
(graph construction) than scatter/dereplicate/count.
- Callback bound is FnMut(Progress) + Send — a third variant, not
matching either prior shape. PartitionRunner::run's on_done is
invoked from its own single controller thread (never concurrently, so
no Sync needed, unlike Dereplicator/Counter's Fn + Sync), but
that controller thread is itself std::thread::scope-spawned, so the
closure still has to be Send to cross into it — caught immediately by
the compiler (cannot be sent between threads safely) when Send was
first omitted, not a design guess. obikalgorithm's eventual shared
trait now has three real callback-bound data points to reconcile
(FnMut alone for PartitionRouter::run's sequential loop, FnMut +
Send here, Fn + Sync for Dereplicator/Counter's rayon
par_iter), not two.
KmerIndex::build_layers deleted outright (KmerIndex stays a pure data
structure — no compute orchestration methods, consistent with dereplicate/
count_kmer's removal in (4)/(6)). New KmerIndex::mark_indexed() added,
symmetric to mark_scattered/mark_counted, replacing the inline
touch(SENTINEL_INDEXED) that used to live inside build_layers.
Stage::start("index")/rep.push(...) and the progress_bar/
"{n} total kmers indexed" log line both moved to cmd/index/mod.rs,
same pattern as (3)/(4)/(6) — LayerBuilder renders nothing itself, just
reports Progress.
All callers updated: cmd/index/mod.rs (Stage 3), obikphylo's test
harness (also gained a mark_indexed() call it was missing — harmless
before since nothing checked IndexState::Indexed in that test, but now
correct).
Full workspace suite green (cargo check --workspace --all-targets +
cargo test --workspace, exit code 0), plus the end-to-end CLI smoke test
(scripts/smoke_test_index.sh, built earlier specifically so this
verification step is a one-liner from now on) — 870 kmers indexed, query
round-trip confirmed, same numbers as (6).
The indexing pipeline is now fully decomposed: obikindexer::
algorithms::{partitionner, dereplicator, counter, layer_builder}, each a
new/(setters)/run algorithm operating on a &KmerIndex (or &mut for
PartitionRouter, not yet fixed — see (5)), KmerIndex itself holding no
pipeline-orchestration logic anymore. Still not done: (5), the future
cache crate, obikalgorithm (now unblocked — three real callback-bound
variants observed, worth revisiting whether a single trait can express
all three or whether that's itself the answer: it can't, and the trait
should not force it).
(8) design agreed, not yet implemented (2026-08-21): obikalgorithm::Algorithm trait + obikindexer::extensions — private/public extension-trait split, KmerLayer rename
Session note: Layer was renamed KmerLayer (user, outside this
conversation, alongside other naming homogenisation with KmerIndex/
KmerPartition) — every reference to Layer in this doc from before
2026-08-21 means today's obikindex::layer::KmerLayer.
Why this came up
Verifying "does cmd/index now rest entirely on the algorithm structs"
(it doesn't quite — see below) led to sorting KmerIndex's own methods by
a criterion the user was explicit is semantic, not mechanical: "les
méthodes qui, sémantiquement, n'ont pas d'intérêt hors de la construction
de l'index" (methods that have no semantic interest outside index
construction) — not "methods only called from cmd/index today," which
a grep could answer but would miss methods construction-adjacent code
elsewhere (merge/select/rebuild/reindex) also depends on for the
same reason.
Checked, not assumed (grepped every call site before classifying):
- Construction-only, real candidates for a private extension trait:
KmerIndex::{mark_scattered, mark_counted, mark_indexed, write_spectrum, build_index_layer, remove_build_artifacts, clear_output_for_create, create_skeleton, finalize_indexed, state}. The last four are called frommerge.rs/select.rs/rebuild.rs/reindex.rstoo (as precondition checks — "is my sourceIndexed?" — or shared skeleton/finalize machinery), not just from the 4-stage pipeline — so this extension trait's scope is "construction of any kind," not narrowly "the initial build pipeline." - Looked construction-only by name, checked, and kept on
KmerIndex:layer_unitigs_path(unitigs are the only way to recover a built index's kmer sequences — read byrebuild_layer.rsand others, well beyond construction — see [[project_unitigs_always_kept]]),pack_matrices(re-runnable maintenance on an already-finished index viaobikmer pack, not just a pipeline step),upgrade_layer_meta(migration, runnable on any existing index at any time).
The general pattern (not obikindexer-specific)
KmerIndex/KmerPartition/KmerLayer stay generic, in obikindex —
every domain-specific consumer crate gets to attach its own extension
trait(s), of two kinds:
- Private (
pub(crate), invisible outside the defining crate) — for plumbing only that crate's own algorithms need.obikindexergets exactly one of these (see below); no public counterpart makes sense for it — "l'index est tellement central que le second trait n'a pas vraiment d'intérêt" for construction specifically: nothing external should ever want to callmark_scatteredorbuild_index_layer. - Public — for a genuinely reusable domain extension. The user's own
example, found while discussing this, not hypothetical:
obikindex/src/ index/distance.rs(phylogenetic distance metrics) is currently animpl KmerIndexblock insideobikindexitself — under this principle it should be a public extension trait owned byobikphyloinstead (distance metrics are a phylo concept,obikindexhas no more business defining them thanobikindex::layerhas defining "family"/"minorant", the reasoningSiblingLayerExtalready followed forKmerLayer— seeobikphylo/src/siblings/iter.rs). Explicitly deferred — noted here so it isn't lost, not part of this round. - The future cache-manager crate (still blocked on (5), see above) will
add its own public extension trait mirroring part of
KmerIndex's/KmerPartition's own read API in cached form (e.g. a cached.partition(i)that doesn't re-touch disk) — same pattern, third data point once built.
Concretely, next to implement (two items, in order)
obikindexer::extensions— a private (pub(crate)) extension trait, most likely named something likeIndexBuildExt(final name not yet chosen), implemented forKmerIndex, carrying the ten methods listed above, moved out ofobikindex::index::{kmer_index, index_layer}. Every algorithm inobikindexer::algorithms::*that currently callsidx.mark_scattered()/etc. keeps the same call syntax (extension trait methods are called the same way as inherent ones, just need the trait in scope) —cmd/index/mod.rsitself would needuse obikindexer::extensions::IndexBuildExt;(or the module re-exports it) to keep compiling, since it's the one place outsideobikindexer's own algorithms that currently callsmark_scattered/write_spectrum/mark_counted/mark_indexeddirectly. Not yet decided: exact trait name, whether it's one trait or split further (e.g. sentinel marking vs. skeleton/finalize machinery), and whethermerge/select/rebuild/reindex(not yet extracted into algorithms themselves) move onto it now too or keep calling the soon-to-be-inherent-no-longer methods some other way in the meantime — ask before implementing, this changes the blast radius significantly (4 moreobikindexinternal files depend onclear_output_for_create/create_skeleton/finalize_indexed/state).obikalgorithm::Algorithmtrait — new crate, the shared traitobikpartitionner→obikindexermerge (session start of 2026-08-21) and (6)/(7) were deliberately building toward, now with four realnew/(setters)/runexamples and three distinct callback-bound shapes to reconcile (plainFnMutforPartitionRouter,FnMut + SendforLayerBuilder,Fn + SyncforDereplicator/Counter— see (7)). Exact shape not yet drafted in this doc — do that as its own design pass before coding, same discipline as everything above.
Both items: design only, nothing implemented yet — this section is the record to resume from, not a plan already executed.
(9) done (2026-08-21): obikindexer::extensions::PrivateBuilder — item 1 above, implemented
Renamed from IndexBuilder to PrivateBuilder immediately after (same
session), freeing the name IndexBuilder for (10)'s public trait — read
IndexBuilder below as PrivateBuilder throughout this section.
Scoped down from (8)'s six-method list to the concrete set that's genuinely movable without further ripple — checked, not assumed, before writing anything:
pub(crate) trait PrivateBuilder {
fn mark_scattered(&mut self) -> OKIResult<()>;
fn mark_counted(&self) -> OKIResult<()>;
fn mark_indexed(&self) -> OKIResult<()>;
fn write_spectrum(&self, f0: u64, f1: u64, counts: &BTreeMap<u32, u64>) -> OKIResult<()>;
fn build_index_layer(&self, i: usize, min_ab: u32, max_ab: Option<u32>, with_counts: bool, mode: &IndexMode, block_bits: u8) -> Result<usize, SKError>;
fn remove_build_artifacts(&self, i: usize);
}
impl PrivateBuilder for KmerIndex { ... }
All six moved bodily out of obikindex::index::{kmer_index, index_layer}
into obikindexer::extensions (new module, pub(crate)) — index_layer.rs
is now empty and deleted outright.
clear_output_for_create/create_skeleton/finalize_indexed/state
stayed inherent on KmerIndex, per (8)'s reasoning: merge/select/
rebuild/reindex — living inside obikindex itself — call them too,
and obikindex can never depend on obikindexer to reach a trait defined
there. Moving those four is real future work (extract
merge/select/rebuild/reindex into algorithms first), not part of this
step.
One new, small, deliberate API widening in obikindex: build_index_layer
depends on three helpers that were pub(crate) to obikindex
(graph_pipeline::{write_graph_as_unitigs, materialize_layer},
common::olm_to_sk) — widened to pub (re-exported from obikindex's
crate root) so obikindexer could reach them. This is exactly the
"enrich shared/lower-level APIs instead of ad hoc local code" call the
project's own rules ask for, made explicitly rather than routed around:
three functions, already generically written (no rewrite needed), now
serve a second caller instead of being duplicated.
Why the trait had to be defined in obikindexer, not obikindex:
Rust's orphan rule — implementing a trait for a foreign type requires
either the trait or the type to be local to the current crate. KmerIndex
is foreign to obikindexer, so the trait must be the local half; if it
were defined in obikindex instead, pub(crate) there would make it
invisible to obikindexer too (crate-private means private to that
crate, not "private except to one named dependent") — the opposite of
what was wanted.
A real design decision made while wiring callers up, not a mechanical
rename: PrivateBuilder being genuinely pub(crate) to obikindexer
means obikmer::cmd::index (a different crate) can no longer call
mark_scattered/mark_counted/mark_indexed/write_spectrum directly —
it never could have, once privacy was real rather than aspirational. Each
algorithm now marks its own completion as part of run()/close()
instead of leaving it to the caller:
- PartitionRouter::close() (not run()) calls mark_scattered() —
close(), not run(), is the actual shared completion point between
the file-driven run() path and the manual write/write_batch+
close() path low-level callers (tests) use; putting it in run()
alone would have silently skipped marking for every caller that never
calls run(). run() already calls self.close() at its own end, so
this covers both paths through one line, not two.
- Counter::run calls write_spectrum then mark_counted before
returning.
- LayerBuilder::run calls mark_indexed before returning.
cmd/index/mod.rs lost all four direct calls (mark_scattered/
write_spectrum/mark_counted/mark_indexed) — each stage's if
idx.state() < IndexState::X { ... } block is now purely "run the
algorithm," no separate bookkeeping call after it. Confirms, precisely
this time (checked by re-reading the whole file, not assumed): cmd/index
now rests on the four algorithms for every read/write of pipeline state
except KmerIndex::{exists, create, state, n_partitions}, which are
genuinely index-identity concerns, not construction bookkeeping — the
original question this whole design pass started from.
Same fix applied to obikphylo's test harness (its four explicit
mark_*/write_spectrum calls removed, relying on the algorithms now
doing it themselves) — obikindexer::algorithms::partitionner's own
pipeline_counts test needed no change (never called mark_* directly).
Full workspace suite green (cargo check --workspace --all-targets +
cargo test --workspace, exit code 0), plus the CLI smoke test — 870
kmers, same as (6)/(7).
Still not done at the time of writing: item 2 from (8) (obikalgorithm::
Algorithm), (5), the future cache crate, the distance.rs →
obikphylo relocation (noted in (8), explicitly deferred), and
extracting merge/select/rebuild/reindex into algorithms.
(10) done (2026-08-21): obikindex::IndexBuilder — the public counterpart, same session
Immediate correction to (9): the private trait built there was renamed
PrivateBuilder (freeing the name), and the four methods (9) had left
inherent on KmerIndex — clear_output_for_create/create_skeleton/
finalize_indexed/state — got their own trait after all: IndexBuilder,
public, defined in obikindex itself (not obikindexer):
pub trait IndexBuilder: Sized {
fn clear_output_for_create<P: AsRef<Path>>(output: P, force: bool) -> OKIResult<()>;
fn create_skeleton<P: AsRef<Path>>(output: P, meta: &IndexMeta) -> OKIResult<Self>;
fn finalize_indexed<P: AsRef<Path>>(output: P, rep: &mut Reporter) -> OKIResult<Self>;
fn state(&self) -> IndexState;
}
impl IndexBuilder for KmerIndex { ... }
User's framing: these four are "maintenance", not "scientific computation
on an index" — a different kind of non-generic-ness than (9)'s six
(mark_*/write_spectrum/build_index_layer/remove_build_artifacts,
exclusive to the 4-stage pipeline). Maintenance is used more broadly
(merge/select/rebuild/reindex), so it gets a real, public trait —
not folded back into KmerIndex's inherent surface, and not private
either.
Where it lives, and why that's not arbitrary: (9) needed the orphan
rule to force its trait into obikindexer, to achieve genuine
crate-private visibility. Here the requirement is the opposite:
merge.rs/select.rs/rebuild.rs/reindex.rs — the trait's own
heaviest users — live inside obikindex. A trait they need to reach
must be local to obikindex (or a crate obikindex itself depends on,
which doesn't exist for this). So IndexBuilder lives in a new
obikindex/src/index/builder.rs, pub trait (no orphan-rule tension at
all here — both trait and type are local to the same crate), re-exported
from obikindex's crate root alongside PrivateBuilder's sibling
obikindexer::extensions::PrivateBuilder staying where it is. Two
traits, two crates, two different reasons, not a contradiction.
Blast radius, all inside obikindex plus one external crate: every
internal caller of these four methods needs the trait imported now that
they're no longer inherent — merge.rs, select.rs, rebuild.rs,
reindex.rs (use crate::index::builder::IndexBuilder;) and, externally,
obikmer::cmd::index::mod (use obikindex::IndexBuilder;, for the three
idx.state() < IndexState::X resumability checks). Call syntax at every
site is unchanged (KmerIndex::create_skeleton(...),
self.state()) — only trait-in-scope requirements are new, which is
exactly the point: same ergonomics, less surface baked into KmerIndex
itself.
Verification went one step further than (9): beyond
cargo check --workspace --all-targets + cargo test --workspace +
scripts/smoke_test_index.sh (all green, 870 kmers again), ran
obikmer merge end to end on two freshly built indexes (exercises
clear_output_for_create/finalize_indexed directly, the two methods
scripts/smoke_test_index.sh itself never touches) — exit 0, pack
stage completed. Test suite alone would not have caught a regression
here: no existing test builds two real indexes and merges them through
the CLI.
KmerIndex itself now carries only: identity/config accessors
(root_path/meta/kmer_size/n_bits/evidence_mode/genomes/...),
path resolution (partition_dir/index_dir/layer_dir/
partition_meta/n_layers), and a few index-maintenance operations not
yet sorted into either trait (layer_unitigs_path, pack_matrices,
upgrade_layer_meta — see (8)'s "tested and discarded" list; still
correctly inherent, not construction-only by the semantic criterion) —
create/open/exists (identity, can't be trait methods needing Self
before one exists) round that out.
(11) done (2026-08-21): KmerIndex/IndexMeta made fully stateless, IndexState moved off sentinel files
Triggered mid-discussion of obikalgorithm::Algorithm (still not started —
see "Still not done" below): user asked why PartitionRouter::new still
took &mut KmerIndex at all, and questioned whether mark_scattered
belonged in the algorithm or in cmd/index. Investigation found &mut
had become newly necessary since (9) — mark_scattered was mutating
self.meta.genomes in memory so Counter's later write_spectrum call
(same idx instance) would see the derived label. User's resolution: the
"disk is truth, stateless" principle already agreed for KmerPartition/
Layer in (5) (still unimplemented for those two) should extend to
KmerIndex itself — move IndexState (Empty/Scattered/Counted/
Indexed) off the three sentinel files (scatter.done/count.done/
index.done, detected by existence) and into a field of index.meta's
own JSON, so the mark_* calls become plain disk writes an algorithm can
legitimately make on &self — no in-memory mutation left to protect.
Shape of IndexMeta, per the user's explicit spec: one JSON file per
index (index.meta), one IndexMeta instance per index, held and
returned as Arc<IndexMeta> (not &IndexMeta) by KmerIndex::meta().
config (kmer_size/minimizer_size/n_bits/with_counts/evidence/
block_bits) is fixed at construction, cached as a pub field (getter
kept alongside, for symmetry) — "les champs constants restent des champs
de la structure", read once, never re-read from disk. genomes and
state are the opposite: no in-memory cache at all, every accessor
(genomes(), state()) re-reads index.meta from disk, every mutator
(push_genome/rename_genome/set_genomes/set_state/mark_scattered/
mark_counted/mark_indexed) does a full read-modify-write of the same
file. An internal std::sync::RwLock<()> is held across each
read-modify-write sequence (not just the write) so two callers sharing
the same Arc<IndexMeta> can't lose an update to each other — this is
not a cross-process lock (that's obisys::DirLock, already held by
cmd/index for the whole build); it only serialises access through one
shared in-process instance.
Construction, chain-of-responsibility style, matching (5)'s pattern:
IndexMeta::create(&KmerIndex, config, genomes) / IndexMeta::open(&KmerIndex)
ask the index for its own root path rather than taking one directly. Since
KmerIndex::create doesn't have a complete KmerIndex yet to hand in
(it's what's being built), added lower-level pub(crate) path-based
primitives create_at(&Path, ...) / open_at(&Path) that KmerIndex::create/
open and builder.rs's create_skeleton call directly, bypassing the
convenience wrappers for that one bootstrap case.
The one deliberate exception: select_in_place and reindex
genuinely rewrite config after an index already exists (output
type/evidence mode changes in place) — contradicting "config never
changes" for the general case. Resolved with a separate, explicitly
rare-labelled IndexMeta::rewrite_config(config, genomes) (preserves
state, overwrites everything else); callers refresh their own cached
Arc<IndexMeta> afterward (self.meta = Arc::new(IndexMeta::open(self)?))
since IndexMeta has no way to reach back into whichever KmerIndex
holds it.
Consequence confirmed, not just hoped for: with mark_scattered no
longer touching anything in memory, PartitionRouter genuinely never
needs &mut KmerIndex — PartitionRouter<'a> { index: &'a KmerIndex },
new(&'a KmerIndex). This is effectively the PartitionRouter half of
(5)'s "order of remaining work" item done as a side effect; KmerPartition/
Layer themselves are still unimplemented for (5).
Blast radius — much larger than (9)/(10), touched nearly every crate:
every .meta().genomes/.meta.genomes field access became a fallible
.genomes()? method call (genomes reads io::Result<Vec<GenomeInfo>>
now, not a field), and .meta_mut() was removed outright (no more direct
field mutation from outside IndexMeta). Fixed across:
- obikindex internals: meta.rs/state.rs/kmer_index.rs/builder.rs
(full rewrites), reindex.rs/select.rs (switched to rewrite_config),
merge.rs (heaviest single file — genome counts precomputed once per
source into a Vec<Vec<GenomeInfo>> up front rather than re-reading
index.meta from disk repeatedly through the function, sentinel write
replaced with dst2.meta.mark_indexed()), stats.rs, distance.rs,
dump.rs, predicate.rs (its IndexMeta-inherent matching_genome_indices/
build_group_filter now read genomes fresh internally), mod.rs/lib.rs
(sentinel constant re-exports removed — IndexState no longer has
SENTINEL_*/detect() at all).
- obikindexer::extensions::PrivateBuilder: mark_scattered signature
dropped &mut self → &self; the four mark_*/write_spectrum bodies
became one-line delegations to self.meta().mark_*().
- obikphylo::siblings: alignment.rs/cardinality.rs/distance.rs/
entropy.rs/sankoff_bundle.rs/stats.rs/tests.rs — all had
self.meta().genomes.len()-shaped reads, mechanically fixed to
.genomes().map_err(OKIError::Io)?.len() (tests: .unwrap()).
- obikmer::cmd::*: annotate (rewrote its rename path to load genomes
once, mutate the in-memory Vec, then idx.meta().set_genomes(...)
instead of meta_mut()), filter/pack/dump/unitig/merge/select/
phylo (fetch-once-then-use pattern for genome counts/labels),
utils/maintenance.rs (run_rename now calls the pre-existing
IndexMeta::rename_genome, dropping its own hand-rolled field mutation
entirely), index/mod.rs (three idx.state() < IndexState::X
resumability checks needed a fallible read — factored into a small
current_state(&KmerIndex) -> IndexState helper rather than repeating
the same unwrap_or_else three times), query/* (emit_batch's
signature changed from &IndexMeta to &[GenomeInfo], and genomes is
now fetched once in run() and threaded down through process_chunk
as Arc<Vec<GenomeInfo>> rather than re-reading index.meta from disk
on every chunk — a deliberate deviation from the "always re-read"
default, justified because this is a genuine per-chunk hot path, unlike
every other call site touched in this pass).
- One &IndexMeta-vs-Arc<IndexMeta> argument-type mismatch pattern
recurred at several CLI call sites (build_filters/build_specs/
emit_batch's original signature) — resolved via Arc's deref
coercion (&idx.meta() coerces to &IndexMeta) rather than changing
every downstream signature to accept Arc<IndexMeta>.
Verification: cargo check --workspace --all-targets and
cargo test --workspace both green (0 failures) after the full
propagation, scripts/smoke_test_index.sh green (870 kmers, same as every
prior round), plus a manual CLI run of index (×2) → merge → select
→ reindex → utils --new-label (rename) → utils --stats, all exit 0,
confirming the four most-affected commands (the ones (10)'s verification
already flagged as under-covered by the automated test suite) still work
end to end against the new Arc<IndexMeta>/on-disk-IndexState shape.
Still not done: (5)'s KmerPartition/Layer self-naming redesign itself
(only the PartitionRouter-&mut-removal piece landed, as a side
effect); the distance.rs → obikphylo relocation ((9), explicitly
deferred); the future cache-manager crate. (8)'s obikalgorithm::
Algorithm trait, resumed and closed in (12) below.
(12) done (2026-08-21): obikalgorithm::Algorithm — the shared trait, resumed and closed in one session
Resumed (8)'s point 2 through a point-by-point discussion of what's
actually common across the four pipeline algorithms, now that (11) made
KmerIndex itself immutable everywhere. Four sub-points, each closed
before moving to the next:
1. Receiver (&self vs &mut self) — investigated whether (11)'s
removal of &mut KmerIndex also removed the need for PartitionRouter::
run to take &mut self. It didn't: PartitionRouter holds real
per-run state of its own (writers: Vec<Option<SKFileWriter>>, open file
handles, purely in-process RAM — confirmed by checking where writers is
stored, nothing to do with KmerIndex/disk truth), unrelated to the
index. First proposal (wrap writers in RefCell so all four could
share a uniform &self) was retracted on pushback: manufacturing
interior mutability with runtime borrow checks to satisfy a cosmetic
uniformity that Rust doesn't even require is over-engineering — a trait
method's receiver must match exactly across implementors, but nothing
stops that shared receiver from being &mut self with three of the four
implementations simply not using the mutability. Settled: trait declares
&mut self; Dereplicator/Counter/LayerBuilder (previously &self)
now also take &mut self, unused.
2. path_source as a PartitionRouter setter, not a run() param —
added a files: Option<Box<dyn Iterator<Item = PathBuf> + Send>> field +
.files(impl Iterator<Item = PathBuf> + Send + 'static) -> Self setter
(boxed rather than a generic type parameter on PartitionRouter<'a>:
negligible cost — one PathBuf per input file, not per k-mer — for a
much more usable type when passing the builder around). run now does
self.files.take().ok_or_else(...), erroring if .files(...) was never
called, instead of taking path_source as a parameter.
3. Unifying the three progress-callback bound shapes — reopened, then
resolved differently than (8) originally framed it. First proposal
(force everything to FnMut(Progress) + Send) was rejected on the same
principle as point 1: Dereplicator/Counter's Fn(Progress) + Sync
isn't arbitrary — their callback is invoked concurrently from multiple
rayon worker threads, and FnMut requires exclusive access, so forcing
it would mean wrapping the callback in a Mutex for zero benefit at the
one real call site (pb.inc(1), already thread-safe). The actual
resolution: move on_progress off run()'s signature entirely, onto a
per-algorithm .on_progress(...) setter — same treatment as point 2's
path_source — so each algorithm keeps its own bound (PartitionRouter:
FnMut(Progress) + 'a, sequential, no Send needed; LayerBuilder:
FnMut(Progress) + Send + 'a, crosses into PartitionRunner's
thread::scope-spawned controller thread once; Dereplicator/Counter:
Fn(Progress) + Sync + 'a, invoked concurrently from rayon workers).
This also dissolves the original problem run() had: once the
callback isn't part of run's signature at all, there's nothing left to
unify there, and point 4 (below) becomes trivial.
4. Output as an associated type, Error fixed — trivial once (3)
moved the callback out: Error was already uniform (all four return
obiskio::SKResult<T> = Result<T, SKError>), only Output varied
(()/()/KmerSpectrum/usize). First cut reused obiskio::SKResult
directly as the trait's return type — caught and corrected the same
session: SKError enumerates I/O-specific cases (BadMagic/
Truncated/Compression/...), meaningless at the level of a generic
"algorithm" abstraction, and borrowing it made obikalgorithm — meant to
be minimal and neutral — depend on a low-level I/O crate purely to reuse
its error type. Textbook instance of the "petits pois" failure mode
(patch around a convenient existing type instead of asking what this
crate should actually own). Fixed to a genuinely generic, boxed error
type owned by obikalgorithm itself:
// obikalgorithm — no dependency on obiskio or any other crate
pub type Error = Box<dyn std::error::Error + Send + Sync>;
pub type Result<T> = std::result::Result<T, Error>;
pub trait Algorithm {
type Output;
fn run(&mut self) -> Result<Self::Output>;
}
Any concrete error (SKError, std::io::Error, ...) converts
automatically via ?, through std's own blanket impl<E: Error + Send
+ Sync> From<E> for Box<dyn Error + Send + Sync> — no custom From impl
needed, no dependency on the crate that defines the concrete error type.
The four algorithms' run bodies needed no change beyond the signature's
return type (every existing ? on an SKError-returning subcall keeps
compiling, converting through the same blanket impl at the boundary).
PartitionRouter/Dereplicator/Counter/LayerBuilder each impl
Algorithm for X<'_> { type Output = ...; fn run(&mut self) -> obikalgorithm::Result<...> { ... } }
— the old inherent run methods were removed outright (not kept as
duplicates), so callers now use obikalgorithm::Algorithm; to call
.run(). Every field-lifetime-bound boxed callback (Box<dyn
FnMut(Progress) + 'a> etc.) is tied to the algorithm's own 'a (the
&'a KmerIndex lifetime already on the struct), not 'static — avoids
forcing callers' progress closures to move-capture (and therefore clone
or Arc-wrap) local state like TracedBar/EMA-rate accumulators that
they'd otherwise want to keep using by reference after run() returns.
Why a new crate, not a submodule of obikindexer: obikmer::cmd::
index::mod and obikphylo's own test helpers both need to call .run()
on these algorithms, so the trait has to be reachable from outside
obikindexer — putting it in obikindexer itself would work file-wise
but conflates "the trait every algorithm implements" with "one crate's
particular four implementations of it", the same reasoning that already
separated obikindex (data model) from obikindexer (algorithms
operating on it). obikalgorithm has no dependencies at all (see
above); obikindexer, obikmer, and obikphylo (dev-dependency, for
its test helper) all depend on it.
Blast radius: obikindexer's four algorithm modules (struct field +
setter + trait impl each); obikmer::cmd::index::mod (three call sites:
.on_progress(cb) before .run(), unqualified now that the trait is in
scope); obikindexer::algorithms::partitionner::tests and obikphylo::
siblings::tests (both had direct .run(None::<fn(Progress)>)-shaped
calls needing the same treatment). New obikalgorithm crate registered
in the workspace Cargo.toml, depended on by obikindexer/obikmer
(regular) and obikphylo (dev).
Verification: cargo check --workspace --all-targets and cargo test
--workspace both green (0 failures), scripts/smoke_test_index.sh green
(870 kmers, same as every prior round) — this round didn't repeat the
manual merge/select/reindex CLI exercise from (11), since nothing
in this pass touched those commands' code paths (only the four pipeline
algorithms and cmd::index, already covered by the smoke test). Reverified
after the obiskio-dependency fix above (same three checks, still green,
obikalgorithm/Cargo.toml now has zero [dependencies]).
Still not done: (5)'s KmerPartition/Layer self-naming redesign; the
distance.rs → obikphylo relocation ((9), explicitly deferred); the
future cache-manager crate (mentioned in (8) as a later, mirrored
extension-trait exercise, not started).
The problem
Reading a layer's data (MPHF + matrix) is not free: MphfLayer::open mmaps
mphf.bin plus (evidence.bin/fingerprint.bin + unitigs.bin), and the
matrix side mmaps matrix.pbmx/matrix.pcmx (or one file per genome column
if not yet packed). Any code path that reopens a layer per lookup instead of
once per run pays this cost repeatedly.
obikphylo::siblings::cache::PartitionCache was built to avoid exactly this
for build_sibling_annex/sibling_annex_stats: those commands probe many
partitions, once per source layer, over the whole run. Profiling a real run
showed wall-clock time dominated by repeated open()/mmap syscalls, not
computation — parallelising the naive per-lookup opens spread the cost
across cores without reducing it. PartitionCache::build opens every
partition's every layer once, up front, in parallel, and keeps the handles
alive for the run.
Three independent implementations of the same bundle (historical — (1) fixed this)
As of 2026-08-20 this table describes the pre-(1) state. Mat no
longer exists (deleted when obilayeredmap::Layer replaced it — see "(1)
done" above); Layer<D> in the table below is what's now called
TypedLayer<D>. QueryLayer is unaffected and still stands as described —
still uncached, still not using Layer at all — which is exactly what (2)
needs to fix. Kept for the original motivation, not as current fact:
Searching the codebase for "who bundles MPHF + matrix, with per-layer format auto-detection" turned up three unrelated implementations:
| lives in | scope | cached? | |
|---|---|---|---|
Layer<D> (now TypedLayer<D>) |
obilayeredmap |
one layer, D fixed at compile time |
held alive by whoever owns the Layer, no policy of its own |
Mat (now deleted; superseded by obilayeredmap::Layer) |
obikphylo::siblings::cache |
one layer, format resolved per instance from an enum of 3 Layer<D> variants |
yes, via PartitionCache |
QueryLayer (unchanged, still current) |
obikindex::query_layer (moved crates since this was written — see "Major restructuring") |
one layer, (MphfLayer, PersistentBitMatrix\|PersistentCompactIntMatrix) pair, bypasses TypedLayer<D>/Layer entirely |
no — opened fresh inside query_partition_with on every call |
query_partition_with is obikmer query's normal query path — the one
most exposed to repeated cross-partition lookups — and it is still the one
with no cache at all. obikphylo built a cache first only because
sibling-annex construction hits the cost hardest, not because the need is
sibling-specific.
The gap in obilayeredmap's existing cache (historical — (1) fixed this)
obilayeredmap::LayeredMap<D> already caches correctly at the granularity
of one partition: open(root) opens every layer once, keeps
Vec<TypedLayer<D>> alive for the LayeredMap's lifetime. But it is
monomorphic — every layer in the Vec must share the same concrete D.
In practice this was false: layers in the same partition are packed
independently over time (pack --sparse converts one layer's presence
matrix at a time). This motivated (1) — obilayeredmap::Layer, done — but
note the specific PersistentSparseBitMatrix-mixing scenario described
here turned out to be moot: PersistentBitMatrix itself absorbed sparse
storage as a 4th internal variant before (1) was built (see "One bug found
… one earlier claim retracted" below), so the only heterogeneity Layer
actually needs to represent is Count vs. Presence, not dense-vs-sparse
presence. LayeredMap<D> itself is unaffected by any of this — it's still
monomorphic, still not used by Layer/KmerPartition (which bypass it
entirely, opening each TypedLayer<D> directly, the same way Mat did).
Resource cost: mmap does not hold a file descriptor
Before deciding how many layers/partitions a cache may hold open simultaneously, the binding constraint needs to be identified correctly.
Confirmed against upstream documentation, not inferred from behaviour:
"After the mmap() call has returned, the file descriptor, fd, can be closed immediately without invalidating the mapping." — mmap(2), man7.org
"The close(2) function does not unmap pages" — mmap(2), Apple Developer
"A file backed Mmap ... will remain valid even after the File is dropped. ... the Mmap handle is completely independent of the File used to create it." — memmap2::Mmap, docs.rs
Every read-only mmap in this codebase already follows this: Mmap::map(&File::open(path)?)?
— the File is a temporary, dropped (fd closed) immediately after the
mapping is established; every persistent struct (PersistentBitVec,
PersistentCompactIntVec, PackedBitMatrix, Evidence, FingerprintVec,
...) stores only the Mmap, never the File. So a cache built on these
types does not consume the process's open-file-descriptor budget
(ulimit -n, notoriously low by default on macOS) proportionally to how
many mmapped files it holds.
It does consume a different resource — the process's virtual-memory mapping
table (one entry per active mmap() region). Linux exposes this as
vm.max_map_count (default 65530). No documented macOS equivalent (fixed
numeric ceiling) was found; the constraint there appears to be virtual
address space rather than an explicit mapping counter, but this is not
sourced and should not be assumed. This is the resource actually worth
measuring before deciding on cache size, not fd count — and it is why
packing (matrix.pbmx/matrix.pcmx, one mmap for all columns) matters
independently of any caching decision: an unpacked Columnar matrix opens
one mmap per genome column, multiplying the mapping count a cache would
have to hold by n_genomes.
Layering: who owns what (superseded — see Definitions above)
This section used to argue nobody owned "the collection of partitions."
That's resolved: KmerIndex (obikindex) owns it now, directly (see
"Major restructuring" below). What's still genuinely unowned is one
partition's open layers — KmerPartition, in the not-yet-created
obikpartition — see "Definitions" at the top of this file for the
current, authoritative answer. Left here only so old links/references to
this heading don't 404; don't read this section for current facts.
Direction agreed, not yet implemented
Only (2) remains — (1) shipped as obilayeredmap::Layer (see "(1) done"
above). Concretely, in order:
- Create the
obikpartitioncrate (obikindex → obikpartition → obilayeredmap, no other edges — see "Definitions" above for the exact constraint and why). KmerPartition { layers: Vec<obilayeredmap::Layer> }—open,n_layers,layer(i),find, plus whatever batch-lookup surfaceobikphylo::siblings::cache::PartitionCachecurrently needs (find_presence_batch/find_presence_batch_fast;fast_modeis sibling-specific bookkeeping and should probably stay inobikphylo, wrapping aKmerPartition/Vec<KmerPartition>rather than living inside it — same "generic vs. domain-specific" splititer_minorants_batchalready went through forLayerin (1)).- Migrate
obikphylo::siblings::cache::PartitionCacheto holdVec<KmerPartition>instead ofVec<Vec<Layer>>. - Migrate
obikindex::query_layer::QueryLayer/query_partition_withto useKmerPartitiontoo, closing the "no cache at all" gap onobikmer's normal query path (see "Three independent implementations," historical, above).
Open before implementing: exact API shape of KmerPartition (propose,
confirm before coding — non-trivial), and whether the multi-partition
Vec<KmerPartition> needs an eviction policy or can simply hold every
partition open for the process lifetime (revisit once the
VM-mapping-count question above has a real number behind it for this
codebase's scale — still not measured).
Preparatory work done (2026-08-20)
Groundwork for (1)/(2), landed ahead of the design itself. Note: at
the time this was written, the collection type these bullets describe was
named KmerPartition (singular) in this doc; it was renamed
KmerPartitions (plural) shortly after, then deleted entirely and merged
into KmerIndex (see "Major restructuring" above). The bullets below are
edited to say KmerPartitions throughout, to not collide with the
unrelated, brand-new singular KmerPartition defined at the top of this
file — the accessors described here live on KmerIndex today, not on
any type called KmerPartition.
KmerPartitions(obikpartitionner, at the time) gainedpartition_dir/index_dir/layer_diras the single source of truth for a partition's on-disk layout, replacing per-module duplicatedconst INDEX_SUBDIR: &str = "index"(7 copies) and ad hoc path joins — including one found duplicated inside the struct itself (ensure_writerrebuiltpart_dir's own logic by hand).- Same struct gained
partition_meta/n_layers/index_mode, wrappingobilayeredmap::meta::PartitionMeta::load(via the existingcommon::load_meta, which also recovers indexes built beforemeta.jsonexisted). Before this,obikphyloandobikindeximportedobilayeredmap::meta::PartitionMetadirectly and called::load()themselves at 21 call sites, each redoing its own error-mapping — every one of those crates knew the on-disk metadata format instead of going through an interface. Fixed everywhere except one remaining spot (below). Caught as a side effect:dump_layer.rs/query_layer.rshad been callingPartitionMeta::loaddirectly, bypassingload_metaentirely — they never got the missing-meta.jsonrecovery the other callers did. - Layer introspection API discussed but not yet implemented — three axes, deliberately kept separate after an initial draft conflated them:
LayerContent { Count, Presence }— what the layer stores; aconstonLayerData(compile-time, zero-cost), not a runtime field.StorageKind { Implicit, Columnar, Packed, Sparse }— how it's stored; only meaningful forDthat actually carry data (Layer<()>has neither this norLayerContent— it's a write-time-only state, never a queryable content: once a layer is closed, "no matrix file" reads back asPresence/ImplicitviaPersistentBitMatrix::open's own fallback, not as some third "empty" content).EvidenceKind { Exact, Approx, Hybrid }— fromMphfLayer's own already-in-memoryLayerEvidencediscriminant.- Not all
(LayerContent, StorageKind)pairs are legal:Countnever hasImplicitorSparse.
Implemented (2026-08-20). LayerContent/StorageKind/EvidenceKind
now exist, each with two forms:
- A runtime accessor on an already-open value (Layer<D>::content()/
storage_kind()/evidence_kind(), PersistentBitMatrix::storage_kind(),
PersistentCompactIntMatrix::storage_kind(), MphfLayer::evidence_kind())
— reads a discriminant already in memory, zero disk access.
- A lightweight detect()/detect_storage() disk probe that mirrors the
corresponding open()'s own priority order by hand (file-existence
checks only, no mmap) — usable before committing to a D, unlike the
runtime accessors. Exposed per-layer on LayeredMap<D> as
detect_layer_content/detect_layer_storage/detect_layer_evidence
(work regardless of D, since they only use self.root + the layer
index).
StorageKind lives in obicompactvec (owner of PersistentBitMatrix/
PersistentCompactIntMatrix); LayerContent/EvidenceKind live in
obilayeredmap. HasLayerContent/HasStorageKind gate Layer<()> out of
content()/storage_kind() (no matrix, nothing to report), matching the
"empty is transitional" conclusion above. 42 new tests across
obilayeredmap's tests/layer.rs and tests/map.rs; full workspace
suite green (0 failed) after.
Not done: these detect() probes don't yet replace Mat::open's or
QueryLayer::open's own hand-rolled equivalents (still duplicated content/
storage decisions, now a third copy of the same logic to keep in sync)
— that consolidation is (1)/(2)'s job, not this prep step's.
One bug found while reading around this (signalled, not fixed); one earlier claim retracted
obicompactvec::bitmatrix::sparse.rs's module doc says "Not used by any production code path yet" — false sinceobikmer pack --sparse(cmd/pack/mod.rs) is wired topack_sparse_bit_matrixandMat::openalready reads the result back in the sibling-annex path. Stale comment, not corrected.- Retracted (2026-08-20): an earlier pass through this doc claimed
obikpartition::query_layer::QueryLayer::openhad no sparse-format detection and would silently corrupt reads on apack --sparsed layer. False —PersistentBitMatrix(obicompactvec::bitmatrix::persistent) is a 4-way enum (Columnar/Packed/Sparse/Implicit), not 3-way as first read; itsopen()already detectsSparseviapresence/sparse_meta.json, and every method on the type (row,fill_row,nonzero_iter, …) already dispatches all 4 arms.QueryLayer::open'sPersistentBitMatrix::open(layer_dir)call was never the bug. Root cause of the false claim: agrep -n "Implicit\|Columnar\|Packed"used to read the enum definition silently skipped theSparse(...)line because it matched none of those three words — a self-inflicted blind spot from a filtered read, not a fact about the code. Lesson: for apub enumwhose variant list matters, read the definition unfiltered, don't grep for the variant names you expect to find. - One real consequence of that same correction, fixed (2026-08-20):
obikphylo::siblings::cache::Mat::SparsePresence(Layer< PersistentSparseBitMatrix>)was redundant —Mat::Presence(Layer< PersistentBitMatrix>)alone already handles sparse layers transparently, sincePersistentBitMatrixabsorbsSparseinternally. Removed the variant, thepresence/is_multi.prsbprobe inMat::open(now just opensLayer::<PersistentBitMatrix>unconditionally for the non-count case — sparse-vs-dense isPersistentBitMatrix::open's own concern), and every now-single-armed match infind_slot/index_batch/iter_minorants_batch/n_cols/fill_sub_matrix_carries. Full workspace test suite green after, including the 27obikphylo::siblingstests that exercisepack_matrices(true)/sparse throughMat.
Remaining instance of the PartitionMeta-encapsulation problem
obikphylo::siblings::family_scan::scan_layer_families still re-derives
index_dir from layer_dir.parent() and calls PartitionMeta::load
itself, purely to get .mode for obilayeredmap::Layer::open (was
Mat::open, same gap, survived the Mat → Layer swap in (1) unchanged).
Fixing it the way the 21 other call sites were fixed needs more than a 1:1
swap: scan_layer_families only receives a bare layer_dir: &Path, not a
(partition, part, layer) triple, and its single upstream source of layer
paths, sibling_layer_dirs, returns a flat Vec<PathBuf> with the
partition/layer indices already discarded. Fixing it properly means either
having sibling_layer_dirs return (PathBuf, IndexMode) (or (part,
layer)) pairs, or threading &KmerIndex + indices through instead of
paths (not &KmerPartition — that type doesn't exist yet, and once it
does it still won't know IndexMode, which lives on KmerIndex/
PartitionMeta) — and touching every one of scan_layer_families's 8
callers (distance.rs, alignment.rs, cardinality.rs, entropy.rs ×2,
sankoff_bundle.rs ×2, stats.rs). Left alone this round; worth doing as
part of the same pass that builds KmerPartition, since those callers are
exactly the sibling-annex consumers it's meant to serve.