Skip to content

feat(package): assayer, an online-learning dynamically resizing linear risk estimation model - #891

Draft
da2ce7 wants to merge 31 commits into
torrust:developfrom
da2ce7:20260910_assayer
Draft

da2ce7 wants to merge 31 commits into
torrust:developfrom
da2ce7:20260910_assayer

Conversation

@da2ce7

@da2ce7 da2ce7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

An assayer does not decide what to do with a sample. It reports what the sample is made of and how confident that reading is, and leaves the decision to whoever owns it.

That is the idea behind the Assayer. It estimates how much risk a subject carries and how much doubt attends that estimate, and it keeps the estimate free of any particular decision. Turning a belief into an action is a separate, pure step the host calls with its own policy in hand, so the same belief can be spent across a login surface, an API surface and a transaction surface without the model learning three different things.


Summary

This PR adds torrust-assayer to the workspace: an online-learning dynamically resizing linear risk estimation model over the measurement reports Spectral Sentinel produces: linear models that learn from labelled outcomes as they arrive, whose dimension follows the Sentinels as they register and deregister, and whose output is a calibrated risk estimate with the uncertainty the evidence supports. It is one commit on top of #827, so it sits on the MSRV-1.89 floor of ADR-T-011 and follows the per-crate versioning of ADR-T-012.

Three runtime components carry the split between belief and decision, and four calls are the whole application-facing surface:

  • Core — stateful. It holds the Bayesian linear models, assembles features from measurement reports, and answers assess() with a channel-free risk assessment; label() feeds outcomes back and receive_sentinel_report() takes measurement in.
  • Derivation Function — pure. derive_reckoning() takes an assessment together with the host's channel policy and Companion Tracker evidence and returns the decision-layer tags. It holds no state and reads nothing the caller did not pass.
  • Companion Tracker — host-owned. It estimates how effective a challenge has been.

Purity is what makes the separation hold rather than merely describe it: a channel cannot reach back into the model when the channel is only ever an argument.

Writes are serialised; reads are not. Model updates run on a dedicated model-owner thread while readers take lock-free snapshots through arc-swap, so the assessment path every request runs never waits on the learning path. Several models evolve side by side: an operational model for live decisions, a slower-decaying sister for drift detection, a fixed-dimension anchor for baseline comparison, and per-axis outcome models. The label path applies a Sherman–Morrison rank-one update under leverage bounds, recomputes the Cholesky factor when the incremental path has drifted from the exact one, calibrates with Platt scaling, and accumulates drift. Coordinates that leave a deployment are removed by Schur complement marginalisation, which keeps the information the surviving coordinates had borrowed from them instead of discarding it.

Measurement stays with the instrument. The crate reads Sentinel's batch reports and takes its graph identifiers from Mudlark directly; it never reaches past a report into the detector that produced it. That boundary is enforced rather than documented: packages/assayer/ci/lint_assayer.sh fails the build on any reference to Sentinel's detector types or its internal report module, and holds decay arithmetic to the two modules that own it, so a decay constant cannot be re-derived at a call site. The testing workflow gains that script as a step.

Contents

The addition is large but self-contained within packages/assayer.

  • A new torrust-assayer crate (0.1.0), 184 source files and 30 integration-test files, with the four runtime calls as its application-facing surface
  • AssayerBuilder and the AssayerConfig hierarchy, registration types, health queries, lifecycle methods and a metrics catalogue
  • Twenty-one records under packages/assayer/adr/: eighteen decision records, plus three (constants.md, migration.md, conventions.md) that fix the forms the rest cite
  • A specification assembled from parts (docs/spec.md and docs/spec/), five layer documents, a deferral register, two numerical studies and about eighty documents in all
  • About two thousand tests, including integration suites that assert invariants through the public surface only

The corpus is labelled rather than numbered. Every record head, claim and test carries a stable label, and records, documentation and tests cite one another by that label rather than by a number or a position in a file. The labels are plain text and need no tooling to read.

Changes outside assayer

  • Cargo.tomlpackages/assayer added as a workspace member
  • Cargo.lock — 9 entries added for the dependency closure (postcard and its cobs behind the optional serde feature, plus two embedded-io entries that postcard's alloc feature names as weak optional dependencies — the resolver locks them but never activates or compiles them; dashmap with the hashbrown 0.14 it uses; lru; foldhash 0.2, because lru turns on the default features of the hashbrown 0.17 the workspace already holds; and the crate itself); no locked version moved. The existing hashbrown 0.15 entry gains a version qualifier on its foldhash reference because a second foldhash enters the graph
  • AGENTS.md — adds the Assayer's A- cross-reference prefix to the package table
  • .github/workflows/testing.yaml — adds the assayer-lint step after the lint step

Manifest under ADR-T-012

The sibling pins carry version beside path: torrust-mudlark = "1.0.0" and torrust-sentinel = "1.0.0", because cargo publish writes those requirements into the published manifest. Each pre-1.0 dependency names the 0.x line the sources are written against — crossbeam-channel 0.5, faer 0.24, lru 0.18, tracing 0.1 and tracing-subscriber 0.3 — instead of a bare 0, for the reason #884 gave for the root's requirements. Nothing depends on the Assayer yet, so it declares 0.1.0. The crate takes a path-only dev-dependency on itself to turn test-support on for its own test builds; that feature carries the test harness, and a path-only dev-dependency is dropped from the published manifest. cargo publish --dry-run -p torrust-assayer stops at resolution because the siblings are not on crates.io yet, which is the publication order ADR-T-012 documents; torrust-mudlark itself still dry-runs cleanly.

Reviewing this

The best starting point is the public surface:

packages/assayer/src/lib.rspackages/assayer/docs/spec.mdpackages/assayer/README.md

From there, the main implementation path is src/api/ for the four calls, src/model/ for the Bayesian linear models, src/feature/ for feature assembly from reports, src/linalg/ for the rank-one, Cholesky and Schur paths, src/guidance/ for the pure derivation, src/health/ for drift, concordance and legibility, and src/ledger/ for report indexing.

For a focused review, I would look at:

  • the four-call surface and what test-support exposes only to the crate's own tests
  • the purity of derive_reckoning(): state and channel policy only ever arrive as arguments
  • the model-owner thread and the snapshot discipline readers rely on
  • the label path: leverage bounds, the Cholesky recomputation trigger, calibration and drift accumulation
  • Schur complement marginalisation when coordinates leave a deployment
  • the feed-forward boundary the lint script enforces against Sentinel's internals
  • the integration tests that assert invariants through the public surface only

Verification

On the candidate commit: cargo fmt --check clean; cargo clippy --workspace --all-targets --all-features -- -D warnings clean on nightly under the workspace lint table, and clean on stable 1.98 for the package; the package's 22 suites pass on both toolchains (1,943 tests, 14 ignored as multi-minute measurements, none failed); the whole workspace passes (96 suites, 4,313 tests, none failed); ./packages/assayer/ci/lint_assayer.sh passes; cargo machete --with-metadata finds no unused dependency; cargo +1.89.0 check --all-features on the package passes (ADR-T-011).

Stable 1.98 clippy at the pedantic level flagged thirty strict float comparisons in the crate's own tests; they now compare bit patterns, and the negative-zero checkpoint test became stricter for it, asserting the sign bit is preserved rather than a numeric equality that cannot see it.

The Schur marginalisation verifies the retained block's positive-definiteness with a Cholesky factorisation of the block lowered by its resolution floor, rather than with a full eigendecomposition: the same predicate, at a third of a cube instead of an order of magnitude more, with every refusal the spectral test made preserved, and no read of the linear-algebra backend's global parallelism anywhere in the crate — every operation sizes its parallelism from the matrix in front of it. The library test binary runs in about 30 s on the reference machine.

cargo audit reports the same advisory set as develop: the one rsa vulnerability with no fixed release, and no new warning. The crate's serde codec is postcard, chosen for its written and versioned wire specification; the persisted checkpoint and journal layouts are internal and gated by a generation number, so no compatibility is owed to any earlier encoding.

Two configurations the instance cannot honour are refused at construction rather than accepted and silently narrowed: an absence threshold outside 1 to 255, the width of the counter that carries it, and a persistence configuration in a build without the serde feature, which compiles neither the checkpoint nor the journal codec. Both refusals carry builder tests at the ends of the range and under each feature arm.

Since the previous cut the package gained three things. Its test support waits on acknowledgements through one named deadline sized for instrumented runs, so a coverage build measures the same liveness an ordinary run does instead of failing on a wall-clock number chosen for the fast case. A checkpoint whose stored model state disagrees with the width it declares for itself is refused at restore with the model and both figures named, where before it passed the reader's checksum and the structural check and met a conversion that asserts. And the feed-forward lint judges every import statement as a whole, admitting one shape — the crate named once, then permitted flat names — so an alias, a glob, a bare import, an extern crate or a re-export can no longer put the sentinel's surface into scope under a name the path check cannot see. The dependency on the sentinel names the version its manifest declares.

Notes

  • Ships at 0.1.0. The public surface is intentional, but follows pre-1.0 SemVer rules until the crate reaches 1.0.
  • Depends on torrust-sentinel = "1.0.0", the version the sentinel package declares; the pin names a released version because a version beside a path is a publishing statement the workspace build never checks.
  • MSRV 1.89, inherited from the workspace (ADR-T-011).
  • No unsafe code; the workspace lint table warns on it and the crate has none.
  • AGPL-3.0-only, inherited from the workspace.
  • Default features: none. serde is opt-in; test-support is for the crate's own tests.
  • The crate estimates only. The decision is a pure derivation the host calls with its own policy.
  • Assayer docs and records use the A- cross-reference prefix added in this PR.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved lifecycle ordering, threshold truncation, silent persistence disabling, and unsafe deserialization arithmetic can produce incorrect or lost state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Sentinel-backed online risk estimation through the new torrust-assayer crate, including concurrent learning, persistence, lifecycle management, and decision derivation.

Changes:

  • Adds Assayer’s public API, model pipeline, health tracking, persistence, and tests.
  • Adds the Sentinel measurement substrate and shared integration-test support.
  • Registers workspace dependencies and Assayer-specific CI linting.
File summaries
File Description
packages/assayer/** New Assayer implementation, documentation, and tests.
packages/sentinel/** Sentinel implementation and test infrastructure.
Cargo.toml Registers the new package.
Cargo.lock Locks its dependency closure.
AGENTS.md Adds Assayer cross-reference conventions.
.github/workflows/testing.yaml Runs the Assayer architecture lint.
Review details
  • Files reviewed: 44/440 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/assayer/src/api/report.rs Outdated
Comment thread packages/assayer/src/api/builder.rs Outdated
@josecelano

Copy link
Copy Markdown
Member

This package has extensive internal specifications, but it does not yet appear to provide a concise, task-oriented integration path for a consumer — especially an AI coding agent asked to add Assayer to a host application. Could we plan a follow-up with a quick-start or runnable example plus a compact integration guide/skill covering the supported flow (construct → ingest reports → assess → apply host-owned policy → label outcomes), the relevant feature flags, and the responsibility boundaries?

I do not want to prescribe a scenario; an authentication/brute-force API is only one possible direction. Non-blocking for this PR.

@josecelano

Copy link
Copy Markdown
Member

Some context on why I raised the documentation point. The Tracker is likely to be the first consumer of this crate. While handling torrust/torrust-tracker#2143 we deferred rate limiting on the REST API: today an attacker can keep guessing admin tokens without being throttled or banned. Cameron suggested using Sentinel + Assayer to detect abusive behaviour across the tracker's three surfaces — REST API, UDP and HTTP announce/scrape.

Our current thinking is layered: a deterministic per-client gate (e.g. failed-auth counters / governor) for the obvious REST case, and Sentinel + Assayer for the announce surfaces, where "bad" is only visible as behaviour over time rather than per request. That's the integration we'd want a quick-start or example to make easy, and why I asked about the docs. We'll open tracker issues for both parts and link them here.

If you think the tracker's announce path is a poor fit for the model — or a better first scenario exists — that would be very useful to hear now, before we design around it.

A sentinel is not a judge. It stands watch, keeps its bearings, and reports what has changed. Spectral Sentinel measures structure in a positionally organised observation stream, learns what has been ordinary so far, and returns statistical readouts when new observations depart from the learned geometry. It does not decide whether a departure is dangerous, important, or actionable; that policy stays with the host.

The crate builds on Mudlark, which supplies the adaptive spatial substrate: regions that receive more observation volume earn finer resolution while quiet regions stay coarse. Sentinel selects the significant V-tree entries, closes them under G-tree ancestry so every selected cell has a complete ancestor chain back to the root, and scores each incoming batch against a learned low-rank subspace model at every selected scale. A second tier of coordination trackers runs at internal G-tree nodes whose subtrees both contribute competitive cells and scores the cross-cell pattern of the per-cell axes, so a coordinated shift that no single cell would flag still surfaces.

Reports carry measurements only: four scoring axes (novelty, displacement, surprise, coherence), maturity, EWMA baselines with upper-tail clipping, CUSUM drift accumulators with a separate slow EWMA, geometry, contour summaries, health snapshots, and the age of the oldest observation in each batch so a host can account for latency across layers. Every value updates Mudlark with exactly one unit of observation volume and anomaly scores never flow back into the spatial index, which keeps spatial adaptation driven by traffic structure rather than by the detector's own conclusions. Temporal decay is applied only when the host asks.

Subspace evolution uses Brand's incremental SVD, with a naive thin SVD as the fallback for small or numerically sensitive cases and as a debug-mode oracle; the two paths are checked against each other across representative configurations. Newly created trackers warm up on synthetic noise through a deferred staging area, with an optional background warming thread so cell creation stays off the ingest path. Configuration validates into structured errors and warnings rather than panicking.

The package ships with twenty-two decision records, a public API reference, an algorithm document and implementation notes, a Criterion benchmark suite, and about five hundred and seventy tests, including two pedagogy integration tests written to be read end to end as a walkthrough of the public surface. Every record head, claim and test carries a stable label so that documentation, tests and code cite one another by name; the labels are plain text and need no tooling to read.

The manifest states what publication needs and what a lock refresh must not decide on its own. The dependency on the sibling `torrust-mudlark` pins `version = "1.0.0"` beside its `path`: `path` is what the workspace build resolves, and `version` is what `cargo publish` writes into the published manifest, so a sibling pin naming a version the sibling does not declare is a publishing defect the workspace build never notices. Sentinel declares `1.0.0` under that same discipline: a sibling crate consumes its public surface through a `version`-beside-`path` pin of exactly this kind, and the surface set out in the package's API reference is covered by semver guarantees from 1.0.0 onwards, so the version a consumer's pin names is the one this manifest declares. The `faer`, `tracing` and `criterion` requirements name the 0.x line these sources are written against — 0.24, 0.1 and 0.8 — rather than a bare `0`: for a pre-1.0 crate `0` is the widest range cargo can be given, and 0.x is precisely where the ecosystem signals breaking changes, so a bare requirement lets a routine compatible lock refresh carry the crate across an API break with no manifest edit and no review.

Outside the package: the workspace gains the member, the lock file gains sixty-nine entries for the dependency closure (faer and rand_distr, plus criterion and tracing-subscriber for development) and moves no version it already held — two dependency references gain an explicit version qualifier because a second version of that crate now exists in the graph — the package prefix table in AGENTS.md gains the S- prefix, and the README records the workspace floor of 1.89.
The deterministic-reproduction test runs the same seeded computation twice
and requires the two results to agree exactly, not approximately: nothing in
the step draws on randomness, iteration order or timing, so any difference at
all is evidence about the machine. That intent was written as `assert_eq!` on
`f64` values, which clippy's pedantic `float_cmp` denies — nightly's clippy
lets it pass, stable's does not, and the crate carries no lint allowances in
its tests.

The lint is right about the form even though the test is right about the
intent. `==` on floats asks whether two numbers are numerically equal, which
is a weaker and less honest question than the one being asked here; it also
answers `false` for two identical NaNs and `true` for `0.0` against `-0.0`.
Comparing `to_bits()` states the actual obligation — the same bits, not merely
the same value — so the assertion now says what its message has always
claimed, and needs no allowance to survive either toolchain. Both runs are the
same computation on the same seed, so the finer comparison cannot fail where
the coarser one succeeded.

A trailing comment recorded an allowance the crate does not carry; it goes
with the construct that would have needed it.
…sleep

The test asked whether a report's age belongs to the batch that carried the
observations rather than running from the sentinel's beginning. It put that
question as: sleep 200 ms, ingest, and require the reported age to come out
under 200 ms. That reads as a property but is really a premise about speed —
it only distinguishes the two behaviours while an ingest is much faster than
the sleep. Under coverage instrumentation the ingest took 771 ms and the
assertion failed, reporting a defect in an engine that was behaving correctly.

An age scoped to its own batch is bounded by the call that produced the batch,
because both ends of the interval fall inside that call. That bound holds at
any speed and on any machine, and it is what the neighbouring test on the
single-batch case already asserts, so the age is now checked against the
measured duration of the ingest it came out of. The bound against the silence
is kept in the vocabulary the claim uses — the age stays below the quiet plus
the call — which is what excluding the silence means once the call's own cost
is accounted for. It follows from the first bound rather than adding strength;
it earns its place by stating the claim in the terms the claim is written in.

The sleep stays: the question is only interesting after a silence, since it is
the silence that a beginning-of-time age would have swallowed.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Configuration validation, checkpoint restoration, and architectural lint enforcement contain unresolved correctness gaps.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 44/440 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread packages/assayer/src/snapshot/working.rs
Comment thread packages/assayer/ci/lint_assayer.sh Outdated
A geometric noise schedule reported itself disabled on the strength of its floor alone, so `Geometric { root: 450, min: 0 }` — a schedule that asks for hundreds of warm-up rounds at depth zero — called itself silent. The batch-size rule keys on that flag, so it stood down too, and a batch size of zero passed validation alongside it. Construction then ran the warm-up rounds the schedule really did ask for, each against a batch of no samples, and divided by the batch count; the latent mean became a non-number and stayed one, which left every surprise score that tracker ever produced unusable, with a configuration that had been told it was fine. A schedule is disabled only when both its root and its floor are zero, because either alone still produces rounds, and with the flag telling the truth the batch-size rule now fires wherever any depth yields a round.

Every numeric bound in validation was a pair of ordered comparisons, and a comparison against a non-number is false whichever way it is written, so a non-number in any floating-point field was admitted by the very check meant to police it. A forgetting factor arriving that way reached the baseline arithmetic and turned every product and sum after it into a non-number as well, so the damage surfaced far from the field that caused it and named nothing. Each field now refuses a non-number ahead of its bound, the way the schedule's decay already did. Infinite values are deliberately left alone where the interval is one-sided: an infinite clip width is the unclipped configuration this package's own clipping study runs as its control arm, and unlike a non-number it compares correctly against every bound it meets. The two-sided intervals still refuse it, through the bounds they already carry.

The headroom the depth gates imply grows as a power of three, and nothing bounded the gap between the two depths. A gap in the forties overflows the exponent: validation aborted on the overflow instead of returning the faults it promises, and without that abort it compared the budget against a wrapped remainder and admitted or refused arbitrarily. The requirement is now computed as a checked power, and a gap too wide to represent is refused on its own terms rather than reported as a budget shortfall, since the depths are what the host must change and no budget could have satisfied it. The graph makes the same computation twice and both are made saturating, which needs no change to any signature it publishes: a saturated ceiling carries the same verdict into the assertion that already stood there, where a wrapped one carried none. The subtraction that follows saturates for the same reason. One of the two sites is reachable even after the sentinel refuses the pairing, because a budgetless graph skips the headroom assertion entirely and still computes the figure.

The coordinate width was unconstrained, and the root tracker spans all of it. At one dimension the tracker's lone basis vector spans the entire space, so novelty is identically zero and the tracker reports a settled model of everything while modelling nothing — the degenerate case the package documents as excluded, reached through a constructor that returned success. The width is a parameter of the type rather than a field of the configuration, so validation of the configuration alone can never see it; it is judged at construction instead, and collected alongside the configuration's own faults so a host wrong in both respects learns both in one pass. The reset path needs no second judgement, because the width is fixed by the type and this is the only constructor: a width that would be refused could never reach a reset at all.

Two error variants are added, both additive: one for a depth gap whose headroom cannot be represented, one for a coordinate width below the tracker's minimum. No existing variant, type or signature changes shape. The configuration tests gain both ends of each new rule — the schedule pairing that was admitted and the one that is genuinely silent, a non-number in every floating-point field, the infinite clip width that must stay admissible, the widest representable depth gap beside the first that is not, and the width refusal beside the narrowest width that still models something.
… the competitive set

Cell intervals are half-open, which needs one exception at the very top of the domain. Where the coordinate width is narrower than the coordinate type the exclusive bound is a representable value outside the domain and nothing is lost; where the width fills the type, the domain's maximum is the type's maximum and there is no value above it to serve as the bound. The topmost interval then excluded a coordinate that is genuinely inside the domain, so the largest coordinate matched no cell at any depth and reached no tracker. It was not, however, ignored: the spatial layer routed it to a live node and the lifetime counter incremented, so the totals recorded an arrival that no tracker was ever shown, and the two readings of the same batch disagreed. The cell whose bound is the domain maximum now owns that maximum. Every other boundary stays half-open, so no coordinate falls into two sibling cells and nothing is counted twice, and the field documentation on both reported forms of the interval now says where the exception applies instead of describing the bound as exclusive everywhere.

The root was removed from the selection after the top-K cut rather than before it, so it spent a slot it could never use. The root is an ancestor by construction and is filtered out immediately afterwards for being one; while it sits in the field it also wins, because its own intensity is what it accumulated before its first split and the split freezes that figure while both children start from zero. It therefore outranks every real candidate until one of them passes a total that is no longer growing. At a capacity of one — a configuration the validation explicitly accepts — the single slot went to the root and was then discarded, leaving the competitive set permanently empty and no descendant able to become competitive at all. Removing the root before the cut spends every slot on an entry that can actually be selected, and costs nothing at larger capacities, where the same defect quietly cost one slot in every recomputation.

Four tests cover the two ends of each rule: the domain's top coordinate reaching a tracker and an ordinary coordinate still landing in exactly one cell, and a capacity of one selecting a cell beside a larger capacity filling every slot, each checking the root is absent from what was selected.
Cached volumes were refreshed before the newly entered cells were added to the queue rather than after, so every cell joined it holding the zero it was constructed with and kept that zero until some later pass happened to refresh again. The queue is served highest volume first, which makes a cell admitted at zero indistinguishable from a cell with no traffic behind it; with a field of zeroes the order fell entirely to the tie-break. That tie-break took the last of the equal maxima from a map keyed in ascending identifier order, and identifiers are handed out as the tree grows downward, so the cell served first was the newest and deepest one — the exact inverse of the rule the queue exists to serve, that a busy ancestor is warmed before the cells beneath it. The refresh now follows the enqueue, so a cell arrives carrying its volume. The tie-break is settled the other way as well: equal volumes resolve to the smaller identifier, which is what the synchronous drain already did, so the two paths through the same queue no longer disagree about which cell comes next.

A scoring pass in which no competitive cell supplied a score returned before the pruning step instead of pruning against an empty active set. Contexts that had just become stale were therefore left standing, and they were wrong in two ways at once. The health figure went on counting them, so a host reading it saw contexts that no longer described anything. More seriously, a node that became active again later found a model already in place, and the baseline warm-up is skipped exactly when one is present — so the context resumed on a model trained against a group composition that had since dissolved, with nothing in the report to say so. An empty active set is a reason to prune everything rather than a reason not to prune, and the pass now does what every other pass does.

The feed-forward count was checked by projecting both sides into floating point and allowing them to differ by less than one arrival. The projection is lossy by its own documentation, and past the magnitude where consecutive integers stop being separately representable it cannot express a single arrival at all: a total and that total plus one land on the same number, while a total plus two lands two away, so an exactly correct sum can sit further from its projected expectation than the tolerance allows and fail an assertion about arithmetic that was never wrong. The comparison is now made in the accumulator's own domain, adding the same unit delta once per observation that the loop above it added, so the check tests the accumulator against itself. This also removes a lint allowance that only the discarded cast needed. The failure itself is roughly nine quadrillion observations away and is not reachable by a test; what a test can pin, and now does, is the property the choice rests on — that the projection loses a single arrival at that magnitude while the accumulator does not.

Four tests accompany the two reachable fixes: a tie resolving to the shallower cell, a newly queued cell carrying a volume rather than a zero, and a pass with nothing to score leaving no context active beside the populated pass that created one.
…ount

The active tracker counts were read from the analysis set, which is the investment set: it names every cell the selector has decided to pay for, including those still waiting in staging for their noise warm-up. Such a cell has no tracker and cannot have produced anything, so counting it active reported a cell as working for as many batches as its warm-up lasted. Under background warming that is the ordinary case rather than a corner, because the drain no longer happens inside the ingest that created the cell. The counts are now taken over the online cells, each of which carries the competitive flag the last reconciliation gave it, and the ancestor figure excludes the root exactly as before. The analysis-set summary is drawn the same way: its producing-set sizes and its depth, importance and V-depth ranges are computed over the selection intersected with the cells that actually have trackers, so the ranges no longer widen to take in cells no observation has reached. The investment figure keeps counting staged cells, because that is what it is for, and the identity between the two figures now holds rather than being asserted between quantities drawn from different populations. The online reading is a second, additive method on the analysis set rather than a change to the existing one: the summary type is re-exported at the crate root, so altering the shape of the method already there would be a breaking change, and naming the two readings separately puts the choice between the investment set and the producing set in front of every caller — which is the distinction the defect turned on.

The semi-internal count was a literal zero on both health paths, each with a marker admitting it, and the field's own documentation carried a third. Semi-internal nodes are a reachable state — an eviction that removes one child of a pair leaves the parent with a single subdivided half — and they sit on the observation-receiving contour, so the figure was wrong exactly while the structure was being reshaped, which is when a reader would look at it. The graph now answers for its own nodes through an additive accessor that counts them, and all three markers go. The count is a scan rather than an incrementally maintained field: the transitions that create and remove a semi-internal node are spread across splitting, eviction and restoration, and a counter threaded through all of them would have to be right at every site to be worth reading at any. The scan is linear in the live nodes, which the budget bounds. No existing mudlark signature changes.

A cell checked out for background warming stopped counting among the competitive targets being warmed, although the figure's own documentation said in-flight cells were included and a note beneath it conceded they were not. Checking a cell out is how the expensive work is done without holding the lock, not a change in what the cell is, so the figure fell precisely when the work was happening and understated what was in progress by the number of cells actually in progress. The competitive flag is now recorded when the cell is taken, so the count includes it without having to read anything from a cell another thread is holding. Cells already finished and waiting for promotion stay outside the figure, which counts what is being warmed rather than everything staging holds.

Six tests: the active counts matching the online cells while the selection is strictly larger, the reported semi-internal figure tracking the graph's own across a run tight enough to evict, an in-flight competitive cell still counted with an ancestor-only one still not, and the shallower-identifier tie-break beside them.
…orts

The contour snapshot reported the terminal nodes alone as its cell count, while the package's own output record defines the figure as the terminal and semi-internal nodes together. A semi-internal node has one half subdivided and one that still accumulates locally, so that second half receives observations exactly as a terminal cell does and is part of the surface the snapshot describes; the graph's own definition of the contour has said so throughout. The count was therefore short by every half-subdivided node — a shortfall that appears precisely while the structure is being reshaped, since that is when eviction leaves parents with a single child. The record is the authority here and the code follows it, on both the ordinary and the empty report path.

Coordination reports were emitted in the order the bottom-up walk produced them, which is strictly post-order and so puts the root — the shallowest context of all — last. That is deterministic, but it is the order of neither record: the output record describes them as ordered by depth, shallowest first, and the report type's own documentation describes them as ordered by identifier. A reader taking the sequence as a descent from the coarsest scale to the finest had it exactly backwards. Sorting on depth and then on ascending identifier satisfies both readings at once, and is a total order where depth alone is not. The two orderings coincide except where eviction and restoration have recycled identifiers, and there depth is the reading that still means what it says; the record's line gains the tie-break so that it states a total order rather than a partial one, which makes it more precise rather than different.

Two tests: nested contexts arriving shallowest first with ties by identifier, and the contour count equalling the terminals plus the semi-internal nodes across a run tight enough to produce them.
The z-score helper documented a denominator of the square root of variance plus the stability constant, while computing the square root of variance plus the constant — the same words in a different order, and a different number wherever the constant matters, which is exactly the regime it exists for. The package's own definition of this score settles which is right: it places the constant outside the root, and every caller of this helper is the fast baseline that definition describes. So the code is correct and the sentence was wrong. The corrected line now also says why the constant sits where it does: outside the root it floors the deviation the score is divided by, which keeps it in the units of the quantity it guards, where inside it would floor the variance and its effect on the divisor would depend on its own square root. No score changes.

The live-tracker count was documented as the size of the analysis set in three places — the report field, the accessor, and the package's own table of accessors — while carrying the number of cells that actually have a tracker. Those are different quantities whenever a cell is still warming, which is the ordinary state under background warming. The field is not removed, because the report type is published and downstream code constructs it; it is described as what it holds, with a pointer to the figure that does report the analysis set, so a reader choosing between the two is told which is which rather than left to discover it.

Both changes are prose. The behaviour they describe is unchanged by this commit.
… corrective SVD fails, and drop the unread round scores

A geometric schedule reported its root as the most rounds it could ask for, while every depth is lifted to at least its floor. With a floor above the root — a pairing validation accepts — the schedule yields the floor at every depth and the root is never reached at all, so the figure understated what the schedule actually asks for, and understated it everywhere rather than at some extreme. That matters because the figure exists for a host sizing warm-up capacity in advance. The ceiling is now the higher of the two.

The incremental subspace strategy re-orthogonalises its basis through a small corrective factorisation, and when that factorisation failed it returned the basis from before the step, wrapped as a successful result. The field it was returned in is documented as an orthonormal basis, and every caller writes it straight into a tracker's state and then relies on that property; handing back a basis that had not been through the step which establishes it satisfied the signature while breaking what the value promises. The failure is reported instead, which is the same answer the strategy already gives when it cannot handle the dimensions at all, and the dispatcher answers it the same way — by running the dense strategy, which needs no such step. The failure itself cannot be provoked from outside without a hook into the linear algebra, and no such hook is added; what a test can reach, and now does, is the fallback the change routes to, exercised at the coordination tier's width where the incremental strategy declines every step on its own dimensional grounds.

A per-round score vector was accumulated for every warming cell at eleven sites across three warming paths, and read nowhere. Its documentation promised a coordination warm-up at promotion that the code no longer performs — promotion leaves the context to form naturally on the next scoring pass — so the field was a record kept for a reader that had been removed, costing an allocation sized to the whole schedule and a push per round on every cell the sentinel ever builds. Field, writes and pre-allocation go together, and the helper that returned such a vector to two callers who both discarded it now returns nothing. The type carrying the field is not reachable outside this crate — its module is crate-private, whatever the field's own visibility says — so removing it changes no published surface.

Two tests: a schedule whose floor stands above its root reporting the floor as its ceiling, beside the configuration check that admits the pairing; and a declined incremental step still leaving a model that scores finitely and keeps its captured fraction inside the unit interval, which a basis that had skipped re-orthogonalisation would not.
…ion guard says it is in

The coordinate bridge trait was documented as sealed to this crate, and nothing sealed it: a public trait over a public supertrait, which any downstream type can implement. The record describing it said both things at once — calling the trait sealed in one paragraph and telling external consumers to implement it for a custom coordinate type in the next — so the two readings could not both be acted on. Sealing it now would close a trait that has been open since the version was published, which is a change of shape for anyone who has already implemented it, so the claim goes rather than the freedom. What is written instead is what holds: the crate implements the bridge for the two widths it encodes, a downstream coordinate type may implement it too, and such an implementor supplies the conversion for its own width or delegates to whichever of the two impls here covers its bits. The record's own line is corrected to match the sentence it already carried further down.

The degenerate-cell dimension guard was recorded as proposed while every part of it is in the tree: the minimum width constant, the eligibility filter that applies it during selection, the skip during reconciliation, the counter of what was skipped and the figure that reports it. They arrived together in the commit that introduced the package, and the algorithm record has treated the guard as settled throughout. The status now says what the tree says. No other line of that record changes.
At 2^53 the projection's format stops separating consecutive integers, and
the invariant records that by asserting the projection of one arrival more
is the same value as the projection of none. The claim is exactness, not
proximity: there is no tolerance to choose here, because any difference at
all would refute the invariant rather than measure it.

`assert_eq!` on the two `f64` projections states that intent in the one
form clippy's pedantic `float_cmp` denies, and the lint is right about the
form: `==` on floats asks the weaker question, and answers it wrongly for
a NaN against itself and for 0.0 against -0.0. Comparing `to_bits()` asks
for the same bits, which is what the surrounding comment already claims
and what the package uses wherever an exact float equality is the point.
The two projections are the same computation on the same magnitude, so
the finer comparison cannot fail where the coarser one succeeded.

The line compiled only because the roster's nightly clippy does not fire
`float_cmp` here; the lint job upstream runs stable with `-D warnings`,
and refuses it.
The warming worker sleeps on a condition variable whose predicate is two
facts: whether the staging area holds warming work, and whether shutdown has
been asked for. It reads both while holding the staging mutex and keeps
holding it until `Condvar::wait` releases it on the worker's behalf, which is
the whole point of pairing a condition variable with a mutex — the predicate
cannot change unobserved between the reading and the sleeping.

Shutdown broke that pairing. It stored the flag and notified without taking
the staging lock at all, so both could land inside the window the worker holds
open: the store happens after the worker has read the old predicate, the
notification is delivered to a thread that is not yet waiting, and the worker
then sleeps on a fact that has already changed. Nothing changes it a second
time, so the wake-up never comes and the `join` that follows waits for a
thread that will never look again. What a host sees is not an error but a
process that stops, because the join sits in the sentinel's own `Drop`.

The handle now keeps the staging `Arc` and performs the transition under that
lock, so the store lands either before the worker reads the predicate — where
the worker sees it and never sleeps — or after the worker is already waiting,
where the notification reaches it. The notification is sent after the guard is
dropped so the woken thread is not made to block on a mutex the shutting-down
thread still holds. The type parameter is now carried by the staging field, so
the phantom marker that stood in for it goes.

A poisoned staging mutex is taken as it stands rather than refused: the lock
is poisoned only when the worker panicked while holding it, that panic is
already reported by the join below, and refusing here would replace the report
with a second panic raised inside a drop.

The witness runs a thousand spawn-and-stop cycles that do nothing else, which
is the arrangement most likely to put the request in the window, and bounds
the wait so that a lost wake-up arrives as a failed assertion rather than as a
suite that stops.
…tructor

Two defects at the observation boundary, one a panic and one a surface that
cannot be used for what it is published for.

The conversion for the wider coordinate took the requested width at its word.
Its counterpart for the narrower type caps the request at the width the type
actually holds, but the wider one sliced a fixed hundred-and-twenty-eight-slot
array at whatever it was given: a request of a hundred and twenty-nine walked
off the end of the array and panicked, and a request of a hundred and
twenty-eight would have shifted past the width of the value as well. Nothing
in the trait's documentation warned of a precondition, so a host computing its
width from a configured domain could reach the panic without doing anything
the published contract forbade. The width is now capped where it is used, for
both the slice and the shift, and the trait method states the rule it has
always relied on: the requested width is a request, capped at what the
implementing type holds, and an implementation applies the cap rather than
trusting its caller.

The same trait is documented as open to a downstream coordinate type, but its
return type could not be built from outside: the length field is private, the
generic constructor is crate-only, and the one public constructor is fixed to
the wider integer at full width. An implementation for a type whose centred
form has to be computed rather than shifted out of an integer therefore had no
way to produce the value its own signature required. The additions are a
public constructor taking the values and the length, a length accessor and its
emptiness companion. The trait's shape does not change and no existing item
changes shape: these are additions to the published surface.

The constructor refuses a length past the backing array rather than clamping
it, with the precondition documented, which is the reading the same type
already takes of a suffix depth past its own width. A length that cannot be
represented is a miscomputation in the implementation, and a clamped answer
would travel on into the tracker as an observation narrower than the one its
author believed it had built.

Three witnesses: the wider conversion at a hundred and twenty-nine, which
panicked before; a vector built from raw values outside the crate and compared
against what the crate's own conversion produces for the same bits, which
could not be written before; and the refusal at a length past the array.
The summary taken over the online cells filters every figure it reports to
the cells that currently have a tracker, which is right for the producing
sets: they are by definition the online ones. It filtered the investment count
along with them, and that figure is documented as the opposite — the whole
investment, online cells and warming cells together. The one part of the
investment the filter removed is exactly the part that has been paid for and
has not yet produced anything, so a direct caller of the public summary was
handed a count that fell precisely when warming was in progress and rose again
when it finished, while nothing had been invested or released.

The count is now taken from the selection itself rather than from the filtered
view, so the method reports what its field means whoever calls it.

The sentinel still writes its own figure over the top, and that is not a
workaround for the defect above. The selection snapshot can only count the
selection; the sentinel counts the trackers — the online map plus everything
in staging — and the report's field is about trackers. The two readings part
wherever a tracker outlives its cell's membership of the selection, and the
population is the one that is being paid for. The construction site now says
which reading it is and why, instead of describing itself as a figure someone
downstream will adjust.

The witness takes a populated selection, puts one cell online, and requires
the producing count to fall to that one cell while the investment count stays
at the size of the whole selection.
Four witnesses asserted less than their prose promised, and one generator was
the reason for two of them.

The four-bit range generator shifts its nibble to the top of the coordinate.
A nibble of sixteen or more pushes its high bits off the end and lands on the
range sixteen below, so the three sprays that swept zero to sixty-four were
sending four passes over sixteen ranges rather than one pass over sixty-four —
concentrated traffic, four times the weight per range, which is the opposite
of the thin spray the tests are named for and the traffic most likely to build
the structure they then check for bounds. A six-bit generator addresses
sixty-four genuinely distinct ranges, and the three sprays use it; the
four-bit generator now refuses a nibble it cannot represent rather than
aliasing it silently, and so does the anomalous-value generator beside it.

The node-budget witness allowed twice the budget plus two. The budget is a
hard ceiling the structure enforces by eviction, and its own invariant check
refuses a node count above it, so the doubled guard admitted every state
between the ceiling and twice it — states the structure itself calls
violations. The bound asserted is now the budget.

The spray-survival witness asked whether any cell or ancestor report carried
samples. The root is an ancestor report, it contains every coordinate, and the
final batch is entirely the concentrated range, so the predicate was satisfied
by the root no matter what had happened to the range it was about: the test
would have passed with every cell of that range evicted. It now requires a
report below the root whose interval contains a value from the range, and
names the reports it saw when it does not find one.

The report-ordering witness asserted that cross-cell contexts ascend by node
handle. The contract is shallowest first with ties broken by the handle, and
the walk is sorted that way; handles are recycled as cells are evicted and
restored, so a correctly ordered run can carry a lower handle at a greater
depth and the old assertion would have rejected it. It now asserts the
documented order, and the claim it carries states both orderings rather than
flattening them into one.
Six documentation statements that a reader could act on and be wrong.

Two interval bounds were documented as plainly exclusive. They are exclusive
everywhere but the top of the domain, where the entry whose bound is the
coordinate maximum owns that maximum, because a width filling the coordinate
type leaves no value above it to be excluded. The cell report and the cell
inspection already say so; the analysis entry and the coordination report now
say the same thing in the same words. The coordination case is not a corner:
the root context covers the full-width interval on every batch, so a consumer
taking the bound at its word drops the top coordinate from the one context
that is always present.

The selection's own field was named as a producing set. The module above it
defines the producing sets as the online subsets of the investment set, and
the field is the closure of the competitive targets under ancestry with no
online filter at all — the investment set itself. Naming it after its online
subset made the internal model contradict the published terminology exactly
when the two differ, which is while cells are warming.

The plan document for the published API states that the engine is not `Sync`,
and gives the staging mutex as the reason. The crate requires `Send + Sync` of
that type statically, in an integration test and in a compile-time assertion
beside the warming handle, and a mutex is what makes a type `Sync` rather than
what prevents it. The line now states the tested contract and the reason it
holds.

Three references had no referent. The record reference on the conversion
trait's banner was the tree's only section-marked record form; it is replaced
by a citation of the label the record carries, which is the form the reference
policy asks for and the one the shared checker resolves. The duration-budget
record cited a numbered section of another package's testing guidelines with
the number left as a parenthetical placeholder, and no section there states a
per-test duration budget: it takes the form this package already uses for a
record with no specification target, written without naming a file, since
amended record text may not cite one. Two shared test helpers carried bare
mark-and-ordinal banners whose ordinals appear nowhere else in the repository;
the mark and the ordinal go and the descriptive heading stays, which is what
the policy prescribes for a reference that points at nothing.

Four of those removals change what the burn register counts, and the register
is an exact census rather than a ceiling: three rows fall by one occurrence
each and the row for the builders helper leaves, because a row that outlives
the occurrences it counted fails the census. Those four lines are the whole of
this change outside the package.
…inimum

The degenerate-cell record reproduces the minimum-width constant with a doc comment saying that cells at or below the threshold are excluded from the analysis set. The exclusion is strict on one side only: candidate collection admits an entry whose suffix width is at least the minimum, reconciliation skips and counts only a width strictly below it, and the tracker's own defensive assertion is written the same way. A test builds a tracker at exactly the minimum and expects an ordinary rank-one model rather than a refusal. A reader taking the record at its word computes the narrowest tracked cell one bit too wide, and reads the skip counter as covering a case the code never skips.

The record's own reasoning already depends on the boundary being inclusive. It justifies the value as the point at which one residual degree of freedom first exists, and its closing consequence keeps that value as the theoretical minimum which prevents the panic; a value the record excluded could do neither. Only the quoted comment disagreed, and the crate's copy of that comment already states what the filter does. The record now states it in the same words, which is what makes the value a minimum rather than a floor the analysis set sits above.
Three decision records describe cell lifecycle mechanisms that the code does not have, or has in only one of its two modes. Each is a statement an integrator would plan around.

The config-validation record credits the validator with rejecting depth configurations that would let a dimensionless cell into the analysis set. The validator has no such clause and could not usefully carry one: the cutoff it would have to work through bounds V-Tree depth, while the ancestor closure walks the G-tree independently of the V-depth an entry was selected at, so the two are not commensurable and no cutoff value keeps a full-depth cell out. Whether such a cell appears is decided by traffic, which configuration cannot be validated against. The rejection that does happen is the runtime suffix-width filter, applied once when candidates are collected and again at reconciliation, where the skip is also counted for the host. The record now names that filter, and corrects the depth range it derived from the mechanism it had imagined — the bound is one narrower than it stated, because a tracked cell must retain two suffix bits and not one.

The noise-injection record still describes injection as running at the moment the selector creates a cell. That was true when the record was written and the later deferred-warm-up record moved it: creation now enqueues, and the rounds are worked either in line within the same reconciliation or a batch at a time on a worker, depending on one flag. Two further statements in the same record turn on the same mistake — that the sentinel's own generator is the whole of its randomness, and that a seed and identical traffic fix the injection sequence. Both hold on the synchronous path and neither holds on the background one, where a second generator on the worker supplies the noise and the interleaving decides which cell receives which draw. Correcting only the first statement would have left the record contradicting itself twice over, so all three now say which mode they describe.

The deferred-warm-up record promises as a consequence that creation and injection never stall the ingest path. That is the point of the background worker, but the flag which starts it is off by default, and without it reconciliation drains the staging area in line and warms each new cell to completion before the call returns — the stall the record's own context section opens with. The consequence now names the mode it holds in, and says plainly what the default trades for its determinism. The cost model above it gains the same premise, so that its claim of zero creation cost is true of the path it is stated for.
… fixed build

Four documents promise reproducibility without saying what it is conditional on: the crate overview's determinism invariant, the determinism record's first consequence, the published API's determinism section, and the implementation notes' architecture summary. Two conditions are missing from all four, and each is load-bearing for a reader who plans a regression comparison around the promise.

The first is the warming mode. With background warming disabled the sentinel's own generator supplies every noise draw, the drain runs to completion inside reconciliation, and a seed plus a traffic sequence fixes the output. With it enabled the warming worker holds a second generator and takes whichever staged cell leads on volume at the moment it looks, while the main thread is still enqueueing and refreshing those volumes. Which cell receives which draw, and which ingest cycle promotes it into scoring, are then settled by the interleaving. The four sites now say so, and say it in the same words, because a reader who finds two of them should not have to decide which is the weaker claim.

The second is the build. The generator is chosen for speed and states that it is not portable: it picks its algorithm from the target's pointer width, so one seed yields different streams on a 32-bit target and a 64-bit one, and it reserves the right to replace that algorithm in a later release. The determinism record named Rust versions as the axis that has to be held fixed; the axis is the dependency's version, and the compiler's is not the one that decides. Both are covered by holding the build fixed — one target, one set of dependency versions — which is the form the four sites now use.

Neither correction weakens anything the code does deliver, and each names what survives background warming rather than leaving the mode as a bare exception: the graph, the investment set and the ascending-handle report order are all still functions of the traffic alone, so an integrator keeps position-by-position comparability of reports and loses only the values inside them and the cycle a tracker starts contributing on. The API section additionally separates ordering from values, since only the values are conditional, and states the two ways a caller can still diff two runs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Clock inconsistencies, NaN-permissive validation, malformed-checkpoint panics, and checkpoint durability gaps can produce incorrect state or data loss.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 44/442 changed files
  • Comments generated: 7
  • Review effort level: Balanced

Comment thread packages/assayer/src/persistence/checkpoint.rs Outdated
Comment thread packages/assayer/src/types.rs
Comment thread packages/assayer/src/api/builder.rs
Comment thread packages/assayer/src/guidance/mod.rs Outdated
Comment thread packages/assayer/src/persistence/recovery.rs
Comment thread packages/assayer/src/resonance/channel.rs
Comment thread packages/assayer/src/testing/scenario.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Persistence can lose restored or journaled state, and clock and convergence paths currently produce incorrect runtime results.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 44/442 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread packages/assayer/src/persistence/journal.rs Outdated
Comment thread packages/assayer/src/persistence/recovery.rs Outdated
…he host

The background warming thread was created with an expect on the builder's
result, so an environment that refused the thread ended the process. The
refusal is an ordinary one — a process or user thread limit already reached,
an address space with no room for another stack — and it can arrive at a
caller whose configuration is entirely correct, because a thread is granted
by the operating system rather than implied by the values a host supplied.
Ending the process there is precisely what this crate's policy on panics
exists to prevent: the engine is a library inside a long-running service, and
a resource limit is not programmer error.

The handle's constructor now hands the operating system's error back. The
handle is crate-private — the module that holds it is not re-exported, and no
type in it appears in the published surface — so the signature change is
invisible outside the package.

Construction reports the refusal. It is the one moment in a sentinel's life
where the caller still holds an error channel, so a new variant carries the
setting that asked for the thread and the environment's own account of the
refusal, and the message says that the configuration is sound. The variant
arrives alone rather than among the constraint violations, because the thread
is requested only after validation has passed and there is nothing left to
collect it with. The enumeration is marked as one that grows in the same
change: every capability a configuration can ask the environment for is one
more way the request can be refused, and a caller matching exhaustively would
have to be edited for refusals it has no opinion about. No consumer matches
it exhaustively today, so the mark costs nothing and is affordable only until
the package is first published.

Reset has no error channel and cannot acquire one without changing a
published signature, so a refusal there leaves the sentinel without a thread
and warns. That is a complete fallback rather than a half-state: the warm-up
dispatch keys on whether a thread is present, not on the flag that asked for
one, so every staged cell is drained inline and every report is produced as
it would have been. What the host loses is the latency the mode was enabled
for; what it keeps is the engine, and the stronger reproducibility that
synchronous warming carries.

The forced failure has no deterministic witness — a test cannot make the
operating system refuse a thread without either a new dependency or
exhausting the runner's own threads — so what is pinned instead is the
message a host receives: it names the setting to withdraw and quotes the
environment, which is what tells an operator whether withdrawing it is the
answer or the machine is simply out of threads.
…oth directions

Seeding copied the source's mean and variance unconditionally but raised the
receiver's warmth only when the source was already warm, so a receiver that
had learned something and was then seeded from one that had not kept its
warmth over the placeholder pair it had just been handed. Every later batch
was then clipped against a ceiling derived from those placeholders and scored
as a departure from them, and the cold path that exists to replace exactly
that state — adopting the first real batch outright instead of decaying
towards it — could never run again, because the flag it keys on had been left
standing.

Warmth is not a separate fact about the receiver. It says whether the two
numbers beside it were measured or are the pair a fresh baseline starts from,
so it belongs to the baseline being transferred and travels with it. Taking
the source's warmth as it stands is both the simpler statement and the only
one under which the method's own description — that the receiver takes the
source's baseline — is true of everything the method moves.

The one caller inside the package cannot reach the case today. The receiver
is an axis' slow baseline and the source is that axis' fast one, and the two
warm together: a scoring pass hands both the same retained batch in the same
call, and the only path that returns either to cold clears them as a pair.
That settles where the defect can be observed, not whether it is one — the
type is part of the published surface and the method with it, so a consumer
seeding one baseline from another is entitled to a receiver that describes
what it actually holds. The change strictly generalises the behaviour the
existing tests pin, and the case that was unreachable now has a witness of
its own: a warm receiver seeded from a cold source comes back cold, keeps the
placeholders, and takes the cold path on its next batch.
…s they state

Four statements about reproducibility and report order claimed more than the
tests behind them established, or less than the engine actually keeps.

The identity test said two runs produce identical reports and compared three
vector lengths, one health counter, two score means per competitive cell and
one per ancestor. Two runs that had modelled different regions of the domain,
at different depths, with different ranks, and that disagreed on surprise,
coherence, every extreme, every z-score, every baseline, all the drift
evidence, the whole coordination tier, the contour, the summary and the rest
of health would have passed it unchanged, provided they produced the same
number of cells. The comparison now runs through a helper that walks both
reports field by field and compares every number by its bit pattern, so the
test establishes the sentence its documentation opens with. One figure is
held out and said to be held out: the age of a report's oldest observation is
an interval between two readings of the engine's own clock, so two runs
differ in it by construction — what is compared there is whether an age is
present at all, which is the part that follows from the batch.

The coordination-order test asserted strictly ascending handles over a vector
the engine sorts shallowest first with the handle breaking ties. The producer
sorts on that pair, the report type documents that pair, and a second test in
the suite already asserted that pair over the same vector — so the assertion
was one a correct readout could fail, needing only an eviction and a
restoration to hand a deeper context a lower handle. The assertion, the test's
name, its label and the claim it carries now all state the order the engine
keeps, and the claim about handle ordering is left where it is true: over the
two cell lists, which it was written for and which do keep it.

The reproducibility claim in the module documentation was unconditional. It
holds with background warming disabled and on a fixed build — one target and
one set of dependency versions — because the warming worker holds a second
generator and takes whichever staged cell leads on volume at the moment it
looks, and because the generator behind the noise is chosen for speed rather
than portability. The package states the guarantee that way already, and the
module now states it in the same words, and notes that the configuration
these tests share leaves background warming off, so what they exercise is the
guarantee exactly as stated rather than a wider one nothing checks.

Two further sentences said all three report vectors come out in ascending
handle order: one in the same module documentation, contradicted by a test
three paragraphs below it, and one in the package overview's determinism
section. Both now name the two orders separately. The claim label that made
the same generalisation is renamed with them, since a label is read as a
statement wherever it is cited.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Restored runtime registries and identity state are incomplete, lifecycle races can leave hidden dimensions, and malformed matrix dimensions can overflow during deserialization.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 59/488 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread packages/assayer/src/lib.rs Outdated
Comment thread packages/assayer/src/lib.rs Outdated
Comment thread packages/assayer/src/lib.rs Outdated
Comment thread packages/assayer/src/linalg/serde_support.rs Outdated
Floating-coordinate graphs can expose G-tree nodes whose depth is greater than N because Mudlark gates splitting on V-tree depth. Unsigned subtraction therefore panicked in checked builds and wrapped to a large eligible width in release builds.

Compute the remaining width with saturating subtraction so an over-deep cell has zero modeled width and the existing minimum rejects it. The crate test constructs that reachable graph state before exercising selection. This adds test coverage without changing the public surface.
Selection rejects candidates whose suffix is too narrow before reconciliation, so the former increment behind the reconciliation guard could not run. Reports consequently exposed a counter whose documented condition was real but whose value stayed zero.

Count exclusions as the selector builds each current snapshot, carry that count through both analysis summaries, and replace the sentinel reading whenever selection is recomputed. Snapshot semantics count nodes in the graph now instead of repeatedly counting the same node as a lifetime event. The selector stores the count in an additive private field and exposes it internally; no public field or signature changes.

The report test now drives a real small-domain sentinel to narrow cells and compares the report and accessor with the exclusions visible in the graph.
The integer domain has no representable exclusive bound above its maximum. CellReport already describes the implemented exception, but the API table and MemberScore still described every interval as half-open. Give both consumers the same top-of-domain ownership rule so the maximum coordinate is not discarded.

SpanTiming::install returns the timing handle together with the subscriber guard. Destructure both in the usage example so the handle exposes total_ns and the guard remains alive for the measurement. The example stays ignored because its benchmark helper lives in a private, test-only module that external doctests cannot name.

These are documentation corrections only; no public item changes shape.
…r risk estimation model

The Assayer is an online-learning dynamically resizing linear risk estimation model. It estimates risk from the measurement reports Sentinels produce, and how much doubt attends that estimate; it learns online from the labelled outcomes the host reports back; and its dimension is derived rather than configured, and resizes as Sentinels register and deregister. The Assayer measures and does not decide: it keeps the estimate free of any particular decision, and turning one into an action is a separate, pure step the host calls with its own policy in hand. The same belief can therefore be spent across a login surface, an API surface and a transaction surface without the model learning three different things.

Three runtime components carry that split, and four calls are the whole application-facing surface. The Core is stateful: it holds the Bayesian linear models, assembles features from measurement reports, and answers `assess()` with a channel-free risk assessment, while `label()` feeds outcomes back and `receive_sentinel_report()` takes measurement in. The Derivation Function is pure: `derive_reckoning()` takes an assessment together with the host's channel policy and its Companion Tracker evidence and returns the decision-layer tags, holding no state of its own and reading nothing the caller did not pass. The Companion Tracker is host-owned and estimates how effective a challenge has been. Purity is what makes the separation hold rather than merely describe it — a channel cannot reach back into the model when the channel is only ever an argument.

Writes are serialised; reads are not. Model updates run on a dedicated model-owner thread while readers take lock-free snapshots through `arc-swap`, so the assessment path every request runs never waits on the learning path. Several models evolve side by side: an operational model for live decisions, a slower-decaying sister for drift detection, a fixed-dimension anchor for baseline comparison, and per-axis outcome models. The label path applies a Sherman–Morrison rank-one update under leverage bounds, recomputes the Cholesky factor when the incremental path has drifted from the exact one, calibrates with Platt scaling, and accumulates drift. Coordinates that leave a deployment are removed by Schur complement marginalisation, which keeps the information the surviving coordinates had borrowed from them instead of discarding it.

Measurement stays with the instrument. The package reads Spectral Sentinel's batch reports and takes its graph identifiers from Mudlark directly; it never reaches past a report into the detector that produced it. That boundary is enforced rather than documented: a CI lint script fails the build on any reference to Sentinel's detector types or its internal report module, and holds decay arithmetic to the two modules that own it, so a decay constant cannot be re-derived at a call site.

The corpus is labelled rather than numbered. Twenty-one records stand in the package — eighteen decision records, plus three that fix the forms the rest cite — beside a specification assembled from parts, five layer documents, a deferral register, and about eighty documents in all. Every record head, claim and test carries a stable label, and records, documentation and tests cite one another by that label rather than by a number or a position in a file; the labels are plain text and need no tooling to read. About two thousand tests stand across a little over two hundred Rust files.

The manifest states what a lock refresh must not decide on its own. Each pre-1.0 dependency names the 0.x line these sources are written against — crossbeam-channel 0.5, faer 0.24, lru 0.18, tracing 0.1 and tracing-subscriber 0.3 — rather than a bare `0`: for a pre-1.0 crate `0` is the widest range cargo can be given, and 0.x is precisely where the ecosystem signals breaking changes, so a bare requirement lets a routine compatible lock refresh carry the crate across an API break with no manifest edit and no review. The sibling pins carry `version` beside `path`, because `path` is what the workspace build resolves while `version` is what `cargo publish` writes into the published manifest, so a pin naming a version the sibling does not declare is a publishing defect the workspace build never notices. Nothing depends on the Assayer yet, so it declares `0.1.0`. The crate takes a dev-dependency on itself to turn `test-support` on for test builds and only for those: the feature carries the harness the tests use, and a path-only dev-dependency is dropped from the published manifest, so it costs publication nothing.

Outside the package: the workspace gains the member; the lock gains nine entries for the dependency closure — the member itself, `dashmap`, `lru`, `postcard` with `cobs` and both `embedded-io` lines beneath it, a second `foldhash` and a third `hashbrown` — and moves no version it already held, `hashbrown` 0.15.5's reference to `foldhash` gaining an explicit 0.1.5 qualifier and `hashbrown` 0.17.1 gaining the dependency list its default features now pull in, both because a second `foldhash` enters the graph rather than because anything moved; the package prefix table in AGENTS.md gains the `A-` prefix; and the testing workflow gains the feed-forward lint step, so the boundary the package enforces locally is enforced in CI too.

Two configurations the instance cannot honour are refused at construction rather than accepted and silently narrowed: an absence threshold outside 1 to 255, the width of the counter that carries it, and a persistence configuration in a build without the `serde` feature, which compiles neither the checkpoint nor the journal codec.

Since the previous cut the package's persistence keeps what it acknowledges and its arithmetic and validation refuse what they cannot represent. A checkpoint's rename is followed by a sync of its directory, so the name survives a power loss before the journal it supersedes is truncated; a journal record is synced before the label that produced it is acknowledged, where a flush alone reached no further than the kernel; and a journal that fails to parse anywhere but at its truncated tail now fails the restore that would otherwise have reported success over lost labels, leaving the operator to recover or choose a cold start explicitly. A timestamp carry that would exceed the representable maximum saturates instead of wrapping. Every floating-point configuration field is validated against non-finite input, ordered range checks no longer let a NaN through, and the resonance channel refuses one too; the guidance and recovery paths read the configured clock rather than the wall clock, so a test clock governs them as it governs everything else; and the scenario harness declares its tracing guard after the world it spans, matching the order in which fields are dropped. The purity budget the release-profile tests hold the reckoning to is widened to the drift that profile actually produces, so the container job measures the same invariant the debug build does; the test harness's version-poll deadlines route through the one liveness constant its acknowledgement waits already use and sleep between polls instead of spinning, so an instrumented run on a contended runner no longer fails on a number chosen for the fast case; the health output register is regenerated against the document it now lives in; and a documentation table escapes the pipes in its notation so rustdoc accepts it. The configuration witnesses end their block-bodied setters with a semicolon so the package satisfies the lint table it is published under. And the test catalogue's forty-four statements of intent — the promises no test yet keeps — each gain a plan document under the package's plans, written from the specification, the records and the harness around the promise and labeled throughout so the corpus can count them; a report describes the testing scaffolding as it stands, and a concept argued from those plans states what the harness should become. The test code compares floats by their bit patterns again where a strict comparison is what the assertion means.

Since the previous cut a restored instance keeps what it restored. Construction after a restore initialises the runtime registries from the working copy — the sentinel feature slots and the registered outcome axes — so a report for a restored sentinel is recognised and a restored axis can be deregistered; a host's normal post-restore registration of an id the working copy already holds attaches to the restored model instead of extending it, and the per-dimension identity payloads a restore recovers are retained until the host registers that dimension, when registration seeds the identity owner from the payload (graph, competitive cells, cell outcomes, already decayed by the restore) rather than starting empty; the encode closure is the one thing a checkpoint cannot carry, so registration still supplies it. Matrix deserialisation computes its dimension products with checked arithmetic and refuses an overflow as a deserialisation error instead of admitting a wrapped length. The witness that outcome predictions stay outside the derivation no longer compares two independently trained worlds under a tolerance that the optimiser's association could cross: it derives twice from one held assessment, with and without the prediction payload, and requires bit-identical output. And the package's tests state every exact float claim as a bit comparison, so the released toolchain's strict-comparison lint reports nothing under the package.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Persistence and lifecycle races can lose durable labels, hang shutdown, panic maintenance, or retain orphaned state.

Review details
  • Files reviewed: 58/490 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants