Conversation
There was a problem hiding this comment.
🟡 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.
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. |
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 / 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. |
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 10, 2026 13:01
c6b23b7 to
b5e5762
Compare
There was a problem hiding this comment.
🟡 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
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 11, 2026 05:33
b5e5762 to
95c4af1
Compare
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 11, 2026 07:24
95c4af1 to
9494e96
Compare
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 11, 2026 19:31
9494e96 to
665639c
Compare
There was a problem hiding this comment.
🟡 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
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 12, 2026 07:46
665639c to
e3a124f
Compare
There was a problem hiding this comment.
🟡 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
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 12, 2026 15:33
e3a124f to
64653bb
Compare
There was a problem hiding this comment.
🟡 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
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 13, 2026 10:36
64653bb to
969302e
Compare
…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-two records stand in the package — nineteen 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 and thirty 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.
da2ce7
force-pushed
the
20260910_assayer
branch
from
September 20, 2026 16:24
969302e to
3c4c369
Compare
da2ce7
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Source-audit false negatives, conflicting API documentation, an incorrect MSRV claim, and a confounded convergence test remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 3
Open (8)
Resolved since last review (4)
attempt_restorereconstructs and decays…
Comment on lines
+117
to
+122
| let Ok(entries) = fs::read_dir(dir) else { | ||
| return; | ||
| }; | ||
|
|
||
| for entry in entries.flatten() { | ||
| let path = entry.path(); |
Comment on lines
+135
to
+137
| let Ok(content) = fs::read_to_string(&path) else { | ||
| continue; | ||
| }; |
Comment on lines
+139
to
+143
| let mut in_cfg_test = false; | ||
| for (idx, line) in content.lines().enumerate() { | ||
| if line.trim_start().starts_with("#[cfg(test)]") { | ||
| in_cfg_test = true; | ||
| } |
| license.workspace = true | ||
| publish.workspace = true | ||
| repository.workspace = true | ||
| rust-version.workspace = true |
Comment on lines
+53
to
+57
| /// Exact-equality requirement (e.g. bit-identical snapshots). | ||
| /// | ||
| /// Exposed as a field rather than hard-coded `0.0` so that tests | ||
| /// read uniformly: `tol.bit_identical` instead of a bare literal. | ||
| pub bit_identical: f64, |
| | `T-` | Torrust (root crate) | | | ||
| | `M-` | Mudlark | `packages/mudlark/docs/idea.md` | | ||
| | `S-` | Sentinel | `packages/sentinel/docs/algorithm.md` | | ||
| | `A-` | Assayer | `packages/assayer/docs/spec.md` | |
|
|
||
| ## Overview · `sec:assayer:readme-overview` | ||
|
|
||
| The Assayer estimates risk from the measurement reports Sentinels produce, and learns online from the labelled outcomes the host reports back. Its dimension resizes as Sentinels register and deregister — a registration extends every model but the fixed-dimension anchor, a deregistration marginalises the same set — and what it produces is a risk estimate: the Assayer measures, and the decision on the estimate is the host's. The Core combines signals from multiple Sentinels into `RiskAssessment` values. Hosts may then call the pure Derivation Function, `derive_reckoning()`, to turn an assessment plus channel policy and Companion Tracker evidence into decision-layer tags. |
|
|
||
| **Table (The extraction index)** · `tab:extraction:reference-index` | ||
|
|
||
| Every index below is extraction-relative — a position within $\mathbf{g}_s \in \mathbb{R}^{60 + 2 m_s}$, not within the Sentinel's slot. To convert a row to a slot-relative offset, add one for the occupancy indicator (`conv:extraction:offsets`). The four per-axis features are named for the axes in their fixed order: novelty, drift, spread and coordination. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 introduces
torrust-assayer, an online Bayesian risk estimator over detached Spectral Sentinel reports and host-declared signals. It learns from labelled outcomes, reports risk with uncertainty, and changes model width as Sentinels, identity dimensions, and outcome axes enter or leave a deployment. The host supplies the evidence and decides how to act.The series is one commit on
developafter the Sentinel introduction merged: the package as a whole, plus the four paths outside it that wire it into the workspace.Problem and model
A host needs to combine measurements from several sources without confusing an anomaly score with a decision. Assayer keeps the learned estimate independent of channel policy: one
RiskAssessmentcan support different policies without training a different risk model for each channel.The Core owns operational, sister, fixed-subspace anchor, and outcome-axis Bayesian linear models. Operational and sister models use different training populations and forgetting rates; outcome axes predict quantities declared and labelled by the host. Feature assembly preserves Sentinel identity and incorporates spatial outcome history. Learning uses bounded rank-one updates, measured recomputation, calibration, and drift diagnostics. Calibration requires enough evidence; a cold model does not begin calibrated.
A host-owned challenge provider supplies a posterior to
derive_landscape(), a pure transform of the assessment and validated channel policy. It returns a presentation-freeDecisionLandscape;render_resonances()supplies optional display tags.request_labels()ranks pending assessments for further evidence. These functions have distinct roles and are not one four-call application API.The model owner serialises lifecycle changes and learning. Within a label update, scoped helpers can update the operational and eligible sister/anchor groups while the owner updates outcome axes; they join before publication. Model and report snapshots publish independently through
ArcSwap. Snapshot acquisition is lock-free; pending storage, signal caching, and other surrounding structures still synchronise. Long-lived identity-maintenance and Ledger-GC threads accompany the owner, with a checkpoint scheduler when persistence is configured.Contents
The package declares version
0.1.0and inherits the workspace's Rust floor (1.90) and licence (AGPL-3.0-only); it has no default features and no package linking exception.src, 33 undertestsincluding support, and one Criterion target file; 24 integration-test targets: 23 top-level Rust files and thetests/multi_channel/main.rsdirectory target.docs, five layer outlines, an assembled specification, the record and deferral registers, numerical studies, and test plans. The ADR directory contains 22 documents: nineteen decision records and three convention records. These counts describe files, including assembled material, not independent specifications.performancetarget covers assessment, label publication, construction/shutdown, pre-seeding, health, and reference marginalisation. Measurements include completion barriers where the subject is asynchronous.serdeenables the optional postcard/Serde persistence and serialization closure.test-supportexposes test affordances and is enabled by the package's self dev-dependency; a downstream dependency can explicitly enable it too.Changes outside the package
Four paths:
Cargo.tomlgains the workspace member line;Cargo.lockgains the member and its closure, resolved by cargo rather than edited (582 entries become 591:cobs,dashmap, twoembedded-ioversions,foldhash,hashbrown 0.14,lru,postcard, and the package itself; no entry removed, no version moved);AGENTS.mdgains theA-prefix row for the package's records;.github/workflows/testing.yamlgains the package's feed-forward lint step after the workspace lint. Nothing else outsidepackages/assayerchanges.Manifest and publication
The manifest names
torrust-mudlarkandtorrust-sentinelat1.0.0beside their paths; the sibling Mudlark now publishes 1.1.0, which the requirement admits. Breaking pre-stable dependency lines are bounded, includingcrossbeam-channel0.5,faer0.24,lru0.18,tracing0.1, andtracing-subscriber0.3; Criterion uses0.8for development. No other tracked package manifest consumes Assayer. Declared versions establish intended resolution requirements; registry publication is not part of this series.Verification
Every run below executed at this head's tree on the build server, each in its own target directory, after the pushed snapshot was confirmed.
cargo metadata --format-version 1 --locked(lock unchanged either side)./packages/assayer/ci/lint_assayer.shcargo audit(advisory ids at this head equal the base's: RUSTSEC-2023-0071, RUSTSEC-2024-0436, RUSTSEC-2026-0192)cargo fmt --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo clippy --keep-going --workspace --all-targets --all-features -- -D warningscargo test --package torrust-index(default features / all features)cargo test -p torrust-assayer --all-targets --all-featurescargo test -p torrust-assayer --all-targets --all-featurescargo test --doc -p torrust-assayercargo test --release -p torrust-assayer --test outcome_ledger --test assess_integrationRUSTDOCFLAGS='-D warnings' cargo doc --workspace --no-depscargo +1.90.0 check -p torrust-assayer --all-featuresThe package's suites agree exactly on stable and nightly: 25 suites, 2,057 passed, 0 failed, 7 ignored. Doc tests: 20 passed, 14 ignored. The release-profile witnesses: 22 passed, 0 failed, on the first run. The workspace lint's only output is the pre-existing manifest warning about an unused
categoriesfield, which is not a rustdoc or clippy diagnostic.cargo machetewas not available on the build server; the workflow installs it, so it runs in CI.Review notes
The starting points are
src/lib.rsfor actual exports,docs/spec.mdfor the contract, anddocs/adrs.mdfor decision ownership. The implementation path runs throughsrc/api,src/feature,src/model,src/linalg,src/owner, andsrc/resonance;src/guidanceis label selection, not landscape derivation.docs/upstream-apis.mdconcerns upstream consumption, not the host API.The critical checks are per-request snapshot acquisition, acknowledgement versus publication, feature-layout changes, restored state, numerical refusal and correction-discard paths, and the feed-forward Sentinel import boundary. Schur regularisation and fallback do not promise unconditional exact information preservation. The README's layer statements describe present capabilities and name the remaining acceptance conditions.
Since the previous head
The package's test harness has been rebuilt around what it observes rather than the order it grew in. The harness now stands in the library behind
test-support, split into modules that each own one concern — durable projections and durable values, a fork for persistence, oracles, probes, playback, sweeps and fixtures — and the benchmarks have left the unit tests for a Criterion target that the feed-forward lint compiles without running a measurement. The convergence witnesses are split by topic, so calibration and discrimination, lifecycle and identity, and the posterior each converge under their own name instead of inside one omnibus suite. A working copy restored from persistence takes its label clock from the engine rather than from the wall, so the first label after a restore no longer decays against phantom elapsed time.Follow-ups
Companion contract coverage, landscape reproduction, and source-policy work remain named backlog entries in
packages/assayer/docs/plans/backlog.md. The deferral register retains implemented or retired dispositions and currently has no open entry. A source declaration or passing suite alone does not establish that every specification claim is witnessed.