6385425a92
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
jordan
|
6385425a92 |
ranking: make Hot and New age-aware; fix the same gap in three more places
Some checks failed
ci/woodpecker/push/woodpecker Pipeline failed
`score_hot` hardcoded `age_hours = 24.0`, so the divisor in `log10(max(views,1)) / (age_hours + 2)^gravity` was constant across the candidate set and `Sort::Hot` reduced EXACTLY to `log10(max(views, 1))` -- a view-count ranking wearing a recency sort's name. Four built-in profiles use it (`hot`, `for_you`, `following`, `brief`); anyone tuning `gravity` was tuning a no-op. The in-code comment justified this by saying a per-entity `created_at` lookup needs an `EntityId -> created_at_ns` reverse map that "is not built". That was stale, and it was the load-bearing claim: `created_at` has been materialized INTO item metadata on every write since `Items::metadata_with_created_at`, the executor has held an `EntityId -> metadata` map since M6p3, and the replication record carries the materialized map so replicas cannot diverge. No index, storage change, schema change or migration -- the scorer reads the map it already had, exactly the way `read_duration` does three lines away. `Sort::New` used `entity_id as f64`. Wrong twice: it assumed IDs are assigned in creation order, and it used the ID's MAGNITUDE as the base score, so on a catalog of N items the sort contributed ~N against a boost sum in single digits. Recency did not participate in the ranking, it annihilated every boost. Now negated age in hours -- same ordering, boost-comparable scale. Three more instances of the same defect class, found by auditing rather than assuming the report was complete: 1. Both age sorts were missing from `needs_metadata_for_sort`, so a profile with no session and no diversity never loaded the map the fix depends on. 2. Every metadata sort was DEAD on the SEARCH path. Its metadata pre-load was gated on `session_context.is_some()` and never consulted `profile.sort`, AND the `ProfileExecutor` it built never had `with_item_metadata` called at all -- the map it did compute went only to the keyword-hint argument, which the sort scorers do not read. `shortest`/`longest` scored NEG_INFINITY and the alphabetical sorts the missing-title sentinel, for every candidate, silently. 3. Under `ReducedCandidates` load the candidate cap kept the highest entity IDs, correct only while `Sort::New` meant "highest ID". Left alone it would discard the genuinely newest items BEFORE scoring -- wrong only when degraded, the hardest case to notice. Now keyed off the `created_at` index via the new `RangeIndex::top_n_descending`. The decision "which sorts read item metadata" now lives on `Sort` itself as an exhaustive match. It was a `matches!` in one executor while a second executor had its own different copy, which is precisely how a metadata-reading sort came to be omitted from both. MEASURED, not inferred: - Real server, 10 items, equal views, ages 2-20 days: before every score was 0.5 (all-equal set folded to the normalizer's midpoint) and the feed returned oldest-first forever; after, 1.0 -> 0.0 strictly descending, newest first. - `new` with zero signals returns the exact REVERSE of candidate-scan order. - `alphabetical_asc`, `shortest`, `longest` verified end to end with title and duration order both opposing entity id. - Metadata point-read cost at 2,000 candidates (the ceiling: `scan_candidates` caps at `max(limit*10, 200)` and `limit > 500` is rejected): 7.25ms, 3.6us per candidate. Guarded at 250ms. THE BUG REPORT'S CENTRAL PROMISE IS FALSE and the changelog says so. §7 claimed this fix lets a zero-signal corpus rank newest-first so a consumer could delete its workaround. It arithmetically cannot: the numerator `log10(max(views,1))` is exactly 0.0 for 0 OR 1 views, so the age divisor has nothing to scale and every candidate still ties -- confirmed on the live server, all ten scores 0.5. Age-awareness begins at the second view. Fixing cold-start needs recency to be ADDITIVE rather than a pure divisor, which reorders every existing Hot consumer, so it is a separate decision. `sort_hot_zero_view_corpus_still_ties_regardless_of_ age` pins the limit so it cannot be rediscovered by accident. Three existing tests asserted the old entity-ID behaviour. Inverted to assert real recency, not loosened -- and each fixture now makes id order and creation order DISAGREE, because an ordering assertion where the two candidate orderings agree is satisfied by the defect too. Three of my own new tests were vacuous for exactly that reason and were caught by mutation-testing; one was also flaky (it passed in a 12-test run and failed run alone, because retrieval order for exactly-tied vectors is not deterministic). Every new assertion is mutation-proven against the implementation it replaces. Full lib suite 2130 passed. Clippy 66 warnings vs 66 at baseline, zero added. |
||
|
jordan
|
cdbe9cb453 |
Merge remote-tracking branch 'origin/main' (m11/m12 cluster) into m9/m10
Reconciles two independently-developed lines from base
|
||
|
jx12n
|
b55ad70141 |
fix: M0-M10 third-pass remediation — durability, replication, and CLI hardening
Resolves the 142 findings from tidal/docs/reviews/CODE_REVIEW_m0-m10.md across the engine, server, net, and CLI surfaces: - WAL/session-journal durability, checkpoint format, and crash-recovery hardening - Replication shipper/receiver, tenant isolation, and migration paths - Cluster scatter-gather, router, standalone server + health/offload endpoints - tidalctl refactored into command modules with JSON output and WAL-state tooling - Cohort, governance, signal-ledger, and vector-registry correctness fixes - Expanded UAT/integration/durability test coverage across all milestones |