695bb822efee965b32a4f4db8ee1c7635e142e03
7
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  logaritmiskandClaude Opus 5
|
a0c2f78aed |
feat: add the missing trait impls and make #[must_use] consistent
Trait coverage (#76), all additive: History Debug is still absent - see below HistoryBuilder + Debug (it derived Clone but not Debug) Rating + PartialEq (Gaussian had it; Rating is a Gaussian plus three scalars and had none) Event/Team/Member + PartialEq (input value types with no way to compare them, which made round-trip tests awkward) ConvergenceReport + PartialEq `#[must_use]` (#67). The coverage had no rule: `filtered_log_evidence` had it and `log_evidence` did not; `rating` had it and `current_skill` did not; `Rating::with_drift_scale` had it and `Member::with_drift_scale` did not. Now on the types — `EventBuilder`, `HistoryBuilder`, `Prediction`, `Gaussian`, `OwnedGame` — which covers most method returns at once, plus the `History` accessors individually. `EventBuilder` gets a message, because a dropped builder is the worst case in the set: measured, `h.event(1).team(["x"]).team(["y"]).winner(0)` without `.commit()` leaves `time_slices_len() == 0` and every skill `None`, with no warning at all. And `ConvergenceReport`'s `#[must_use]` moves off the TYPE onto `converge_partial`, where its stated reason is true. It read "from `converge_partial` this may describe a fit that stopped at max_iter" but fired on `converge` too — where that is false, since `converge` returns `Err(NotConverged)` in exactly that case. So the crate's own front-page example warned, and every quickstart had to write `let _ =`. Verified from a consumer crate: `h.converge()?;` now compiles clean. Marking the types made eight method-level attributes redundant, which clippy's `double_must_use` caught — that is the type-level marker doing its job, and the eight are removed. Refs #76, #67 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
8dff7513f7 |
fix!: seal ConstantDrift's field so gamma can be validated
`gamma` enters only as `gamma * gamma`, so the sign was squared away: measured against the old public-field form, `ConstantDrift(-0.0833)` produced results bit identical to `ConstantDrift(0.0833)`. The sign was neither rejected nor honoured — it vanished. It could not be checked while the field was a public tuple position, because there was nothing to intercept. Validating inside `variance_for_elapsed` would have been worse: it runs in the sweep, so a construction-time mistake would panic mid-inference, and `Gaussian::from_ms` is a worked example of why that is the wrong place — rejecting NaN there turned the NonFiniteResult reporting path into a crash. So `ConstantDrift::new` is the only way in and it checks, with `gamma()` to read the value back. 129 call sites rewritten across src, tests, benches, examples and the README. The dated plan and spec documents under docs/superpowers are left alone: they record what was built at the time, and rewriting them would falsify that. tests/constructor_validation.rs is the more valuable half. This defect class was closed three times in one session and reopened twice, because each fix validated the layer it had just touched and inferred the rest — `HistoryBuilder`, then `Game`'s own entry points, then the constructors beneath both. A per-site fix cannot notice the site nobody thought of, so that file enumerates every public entry point taking a magnitude and asserts each refuses negative and non-finite values. It found an eleventh defect on its first run: `HistoryBuilder::score_sigma` accepted infinity, because `inf > 0.0` is true and the assert only tested positivity. Fixed, and its own `should_panic` message updated to match. `Gaussian::from_ms` is deliberately exempt from the non-finite half, for the reason above: a broken fit produces a NaN sigma legitimately and `converge` must be allowed to report it. The convergence-level drift-variance check stays and is now tested through a custom `Drift` implementation, since `ConstantDrift` can no longer reach it. That check is the only thing standing between a third-party `Drift` and a NaN fit. BREAKING CHANGE: `ConstantDrift`'s field is private. Replace `ConstantDrift(x)` with `ConstantDrift::new(x)`, and `drift().0` with `drift().gamma()`. `HistoryBuilder::score_sigma` now rejects infinity. Closes #65 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ |
||
|
logaritmiskandClaude Opus 5
|
ab23476aaf |
fix!: validate the constructors below HistoryBuilder
0.8.0 closed the sign-absorption defect at `HistoryBuilder::mu/sigma/beta`
and at both ingestion paths. It was still open one layer down, in the
constructors those paths call. Measured, all bit identical to their
positive counterparts:
Gaussian::from_ms(25.0, -8.33) == from_ms(25.0, +8.33)
Rating::new(_, -4.17, _) == Rating::new(_, +4.17, _)
ConstantDrift(-0.0833) == ConstantDrift(+0.0833)
sigma, beta and gamma enter only as squares, so the sign vanished without
comment. Worst of the set: `Rating::new(_, NaN, _)` reached `Game::ranked`
which returned **Ok** carrying `Gaussian { pi: NaN, tau: NaN }` — no
`converge` on that path to catch it.
`from_ms` and `Rating::new` now reject. `ConstantDrift` cannot: the field
is public and positional, so there is no constructor to intercept, and
sealing it would break every `ConstantDrift(x)` for a case whose resulting
model is perfectly valid. Documented instead. Its non-finite half IS
rejected — `converge` validates the drift variance each competitor
accumulates, which also covers a custom `Drift` impl.
Two things the tests caught that I had wrong:
NaN sigma must PASS `from_ms`. My first version rejected it, and two
existing tests went red immediately: a broken fit legitimately produces a
NaN sigma from `sqrt` of a negative truncated variance, and the design is
to propagate that to `NonFiniteResult`. Rejecting it turned the reporting
path into a panic inside inference. Written as
`sigma >= 0.0 || sigma.is_nan()` so the intent is explicit rather than
hidden in a negated comparison.
Very small sigma is also not rejected, and that is deliberate: `approx`
produces small truncated sigmas legitimately. `pi = 1/sigma^2` leaves
f64's range below ~1.5e-154 and `tau = mu*pi` overflows sooner, at a
threshold that depends on mu — so there is a band where pi is finite and
only tau is not. Both land on the existing point-mass representation.
Documented, including that such a Gaussian is not equal to itself and can
make two identical declarations report as conflicting.
BREAKING CHANGE: `Gaussian::from_ms` panics on a negative sigma, and
`Rating::new` panics unless beta is finite and non-negative. `converge`
returns `InvalidParameter` for a non-finite drift variance.
Closes #61
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
|
||
|
logaritmiskandClaude Opus 5
|
87fca8dcca |
docs: correct drifted documentation and compile the README in CI
Four README code blocks no longer compiled: `Player` was renamed `Rating`
in T2, the `Drift` trait gained a `T: Time` parameter and a second method,
and two blocks were missing imports outright. The `Rating` example needed
more than a rename — with the binding unused, `T` is ambiguous because
`ConstantDrift` implements `Drift<T>` for every `T`, so it now carries an
explicit annotation.
Nothing compiled those blocks. `src/lib.rs` gains a `cfg(doctest)` struct
carrying `#[doc = include_str!("../README.md")]`, which turns every `rust`
block into a doctest without displacing the curated crate docs as the
front page. Verified it bites: reintroducing `Player` fails the build with
E0432 rather than shipping. Illustrative blocks are fenced `text` — note
that a bare fence defaults to `rust` under rustdoc, which is how the
`variance_delta = elapsed * γ²` formula became a compile error.
Prose fixes: README claimed `Gaussian::forget` takes a square root (it
works in variance space) and pointed at a `.gamma()` builder method that
does not exist. CLAUDE.md's data-flow diagram spliced the public ingestion
shape into the internal one — `Team` is not in that chain — listed
`cdf()`/`erfc()` as public when they are `pub(crate)` and private, and
called `SkillStore` public when only `CompetitorStore` escapes the crate.
Rustdoc fixes: `EventBuilder::scores_with_sigma` claimed a debug-assert
that `Outcome::scores_with_sigma` never had and whose own docs contradict;
rejection happens at ingestion as `InvalidParameter`. `event.rs` described
`add_events_with_prior` as replaced when it is still the ingestion
chokepoint. `factors.rs` advertised `Game::custom` without noting it is
`#[doc(hidden)]`. Internal T2/T4 milestone labels are dropped from public
items; the ones in the private `time_slice` module are left alone.
Closes #35
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU
|
||
|
logaritmiskandClaude Opus 5
|
617bc07f6f |
feat: allow drift to vary per competitor via Member::with_drift_scale
Drift was a property of the History, so every competitor drifted at the same rate and a fixed reference point could not share a graph with moving competitors. A bot at a known strength, a rating floor, a course difficulty — all of them drifted along with the players. Member::with_drift_scale(s) multiplies the drift *variance* a competitor accumulates, so s is in the same units as gamma: ConstantDrift(g) at scale s behaves exactly as ConstantDrift(g * s) would for that competitor. A scalar rather than a per-competitor Drift keeps History's single D type parameter untouched and stays Copy. 0.0 pins a competitor still. The scale lives on Rating, beside the drift it scales, and is applied only through Rating::drift_variance_delta / drift_variance_for_elapsed. Making those the sole entry points means a caller cannot reach the raw drift and silently skip a competitor's scale — the filtered pass was exactly that bug during development, caught because its test was written before the wiring. Like with_prior, the scale is competitor configuration captured at first appearance rather than a per-event override; a competitor that is static is static, and a scale that changed between events would make the skill trajectory hard to interpret. Member's docs claimed prior was a per-event override, which the code has never done — corrected here. A negative scale is rejected rather than squared into its absolute value, and a non-finite one rejected outright, both as InvalidParameter. None means 1.0, so no existing call site changes and no existing fit moves. Adding a public field to Member does break struct-literal construction downstream. Closes #34 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU |
||
|
logaritmiskandClaude Opus 5
|
6030dc78de |
refactor: unify convergence defaults, validate builders, clear dead code
Convergence configuration had two disagreeing sources of truth and one
misleading report:
- `EpsilonOrMax::default()` capped at 10 iterations while
`ConvergenceOptions::default()` allowed 30, and which applied depended on
whether inference went through `run_chain` or a `Schedule`. The schedule
default now derives from `ConvergenceOptions`.
- A graph with no iterating factors reported `converged: false` with an
infinite step, despite being at its fixed point after the setup pass. It
now reports converged with a zero step.
- `TimeSlice::iterate_to_convergence` hard-coded an epsilon and a
20-iteration cap matching neither. It reads `self.convergence` and is
scoped to `#[cfg(test)]`, which is all it was ever used by.
`HistoryBuilder::p_draw` and `::convergence` now validate their arguments
like `score_sigma` already did, instead of accepting a negative `p_draw` or
an `alpha` of zero — the latter leaves every EP update unapplied, so
inference silently returns the priors.
Removing the `#[allow(dead_code)]` masks let the compiler report what they
were hiding: four `OwnedGame` fields that were stored and never read, two
`ColorGroups` helpers and three `SkillStore` helpers used only by tests, and
`iterate_to_convergence` above. Test-only items are now `#[cfg(test)]` and
the unread fields are gone.
Also exported `HistoryBuilder`, which was public but unreachable — callers
could chain `History::builder()` but could not name the type — and added
`Rating::{prior, beta, drift}` and `Index::get`, so handles the API hands
out can be read back.
Two goldens moved, both convergence residuals rather than exact values:
`iterate_to_convergence` now runs to 30 iterations instead of 20, landing
nearer the symmetric truth of 25.0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej
|
||
|
logaritmisk
|
d2aab82c1e |
T0 + T1 + T2: engine redesign through new API surface (#1)
Implements tiers T0, T1, T2 of `docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md`. All three tiers have landed together on this branch because they build on one another; this PR rolls them up for a single review pass. Per-tier plans: - T0: `docs/superpowers/plans/2026-04-23-t0-numerical-parity.md` - T1: `docs/superpowers/plans/2026-04-24-t1-factor-graph.md` - T2: `docs/superpowers/plans/2026-04-24-t2-new-api-surface.md` ## Summary ### T0 — Numerical parity (internal) - `Gaussian` switched to natural-parameter storage `(pi, tau)`; mul/div now ~7× faster (218 ps vs 1.57 ns). - `HashMap<Index, _>` → dense `Vec<_>` keyed by `Index.0` (via `AgentStore<D>`, `SkillStore`). - `ScratchArena` eliminates per-event allocations in `Game::likelihoods`. - `InferenceError` seed type added (1 variant). - 38 → 53 tests passing through T1. - Benchmark: `Batch::iteration` 29.84 → 21.25 µs. ### T1 — Factor graph machinery (internal) - `Factor` trait + `BuiltinFactor` enum (TeamSum / RankDiff / Trunc) driving within-game inference. - `VarStore` flat storage for variable marginals. - `Schedule` trait + `EpsilonOrMax` impl replacing the hand-rolled EP loop. - `Game::likelihoods` rebuilt on the factor-graph machinery; iteration counts and goldens preserved to within 1e-6. - 53 tests passing. - Benchmark: `Batch::iteration` 23.01 µs (slight regression absorbed in T2). ### T2 — New API surface (breaking) **Renames:** - `IndexMap → KeyTable`, `Player → Rating`, `Agent → Competitor`, `Batch → TimeSlice` **New types:** - `Time` trait with `Untimed` ZST and `i64` impls; `Drift<T>`, `Rating<T, D>`, `Competitor<T, D>`, `TimeSlice<T>`, `History<T, D, O, K>` all generic. - `Event<T, K>`, `Team<K>`, `Member<K>`, `Outcome` (`Ranked` variant; `#[non_exhaustive]`). - `Observer<T>` trait + `NullObserver`. - `ConvergenceOptions`, `ConvergenceReport`. - `GameOptions`, `OwnedGame<T, D>`. **Three-tier ingestion:** - `history.record_winner(&K, &K, T)` / `record_draw(&K, &K, T)` — 1v1 convenience. - `history.add_events(iter)` — typed bulk. - `history.event(T).team([...]).weights([...]).ranking([...]).commit()` — fluent. **Query API:** `current_skill`, `learning_curve`, `learning_curves` (keyed on `K`), `log_evidence`, `log_evidence_for`, `predict_quality`, `predict_outcome`. **Game constructors:** `ranked`, `one_v_one`, `free_for_all`, `custom` — all returning `Result<_, InferenceError>`. **`factors` module:** `Factor`, `Schedule`, `VarStore`, `VarId`, `BuiltinFactor`, `EpsilonOrMax`, `ScheduleReport`, `TeamSumFactor`, `RankDiffFactor`, `TruncFactor` now public. **Errors:** `InferenceError` gains `MismatchedShape`, `InvalidProbability`, `ConvergenceFailed`; boundary panics converted to `Result`. **Removed (breaking):** `History::convergence(iters, eps, verbose)`, `HistoryBuilder::gamma(f64)`, `HistoryBuilder::time(bool)`, `History.time: bool`, `learning_curves_by_index`, nested-Vec public `add_events`. ## Behavior change (documented in CHANGELOG) `Time = Untimed` has `elapsed_to → 0`, so no drift accumulates between slices. The old `time=false` mode implicitly forced `elapsed=1` on reappearance via an `i64::MAX` sentinel — that quirk is not reproducible under a typed time axis. Tests that depended on it now use `History::<i64, _>` with explicit `1..=n` timestamps. One test (`test_env_ttt`) had 3 Gaussian goldens updated to reflect the corrected semantics; documented in commit `33a7d90`. ## Final numbers | Metric | Before T0 | After T2 | Delta | |---|---|---|---| | `Batch::iteration` | 29.84 µs | 21.36 µs | **-28%** | | `Gaussian::mul` | 1.57 ns | 219 ps | **-86%** | | `Gaussian::div` | 1.57 ns | 219 ps | **-86%** | | Tests passing | 38 | 90 | +52 | All other Gaussian ops unchanged (~219 ps add/sub, ~264 ps pi/tau reads). ## Test plan - [x] `cargo test --features approx` — 90/90 pass (68 lib + 10 api_shape + 6 game + 4 record_winner + 2 equivalence) - [x] `cargo clippy --all-targets --features approx -- -D warnings` — clean - [x] `cargo +nightly fmt --check` — clean - [x] `cargo bench --bench batch` — 21.36 µs - [x] `cargo bench --bench gaussian` — unchanged from T1 - [x] `cargo run --example atp --features approx` — rewritten in new API, runs clean - [x] Historical Game-level goldens preserved in `tests/equivalence.rs` - [x] Public API matches spec Section 4 (verified by integration tests in `tests/api_shape.rs`) ## Commit history ~45 commits total across T0 + T1 + T2. Each task is self-contained and individually tested; the branch is bisectable. See `git log main..t2-new-api-surface` for the full list. ## Deferred to later tiers - `Outcome::Scored` + `MarginFactor` — T4 - `Damped` / `Residual` schedules — T4 - `Send + Sync` bounds + Rayon parallelism — T3 - N-team `predict_outcome` — T4 - `Game::custom` full ergonomics — T4 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #1 Co-authored-by: Anders Olsson <anders.e.olsson@gmail.com> Co-committed-by: Anders Olsson <anders.e.olsson@gmail.com> |