tidaldb/docs/planning/milestone-4/phase-4.md
jordan fe8d0c87e7 harden: restore CI verification, remove four wire-level fabrications, instrument the 401 path
Implements tmp/tidaldb-fleet-hardening (20 planned tasks + 2 found by measurement).

Ring 0 — restore verification. .woodpecker.yaml step pods ran at the namespace
default of 1500m/2Gi, which OOMKilled a prior pipeline and starved the release
gate past its budget. Both push-path steps now declare
backend_options.kubernetes.resources as two YAML anchors declared once on their
first consuming step. The values are CALIBRATED against measured free node
capacity, not against the LimitRange max: `requests: cpu 2` (this roadmap's
original figure) fits on NO node and would sit Pending forever, because
`ci-build-bounds` grants permission and the nodes supply capacity, and those are
not the same thing.

The `nightly` cron described in this file for 216 days was never created, so
tier-3 chaos, the fault classes, mTLS and the PITR test produced exactly zero
signal while reading like standing coverage. nightly-chaos and
nightly-security-ops now alias the anchors and have budgets matching the gate
(their 120/90 were TIGHTER on the same runner, so they would have failed
nightly for a budget reason, not a correctness one). nightly-soak is REMOVED,
not scheduled: it drives 1000 rps for 600s gating on p99 <= 250ms, and the best
node has 1700m free CPU, so it would fail on starvation rather than regression —
manufacturing a nightly false alarm. Its commands move verbatim to
docs/runbooks/nightly-soak.md.

Ring 1 — four fabrications removed from the wire.
- scatter_merge sorted and truncated without re-stamping rank, so /feed and
  /search returned 1,1,2 under full placement. Reuses merge_cross_shard's
  existing stamp; asserted on BOTH the multi-group merge path and the
  single-group [only] fast path that bypasses it.
- aggregate_region_row's None arm invented `applied_events: 0` plus a deficit
  derived from it. applied_events/lag_events are now Option<u64>, null on the
  wire. leader_last_seq was also unwrap_or(0), so a node that could not reach
  the LEADER computed 0 - applied = 0 for every region and reported a converged
  cluster it had never measured — a fabrication pointing the dangerous way.
- tidalctl inferred NO REPORT from `applied == 0 && lag > 0`. That heuristic was
  actively hiding the PVC-wipe shape: a measured zero with a real deficit
  rendered as "no report" instead of BEHIND. Now read off the wire; converged
  exits 0, partitioned still exits nonzero.
- /sharded/* answered 201/204 for single-copy writes with nothing anywhere
  saying so. Now requires `x-tidal-ack: local`, rejecting with 400 via the
  existing invalid_input path. Six call sites migrated, not the two this
  roadmap predicted — including docs/runbooks/cluster.md §16.3, which told
  operators to run a quorum-write probe via POST /sharded/items. That probe
  cannot verify quorum: the surface applies locally with no WAL append. It was
  used as the safety check between every step of a staged deploy earlier today.

Ring 2 — observability. JSON_LOGS was already implemented and the deployment
simply never asked for it; the StatefulSet now sets it, plus
TIDAL_SERVICE_NAME=tidaldb because enabling it silently renames the
VictoriaLogs `service` stream field and would have blinded every query keyed on
it. Adds tidaldb_usearch_replicated_vectors_total, incremented on BOTH the
origin (wal_blob_first -> Ok(Some)) and the follower apply path — counting only
the origin would mean each vector lands on exactly one node, replicas never
agree, and the alert built on it pages forever.

Found by measurement, not planned: the 401 path discarded every fact about
every rejection. Traefik has served 101,858 rejected requests to the public
ingress — 87.6% of all its traffic — with no record of who or why anywhere.
unauthorized_response now emits reason (missing_token vs invalid_token, the
distinction that separates a scanner from a rotation that missed a consumer)
and the forwarded client. The token is never logged.

Also: scripts/restore-fleet.sh --cluster started the soak monitor while
deliberately leaving its gate suspended, orphaning a watcher that has reported
"0/30 green nights" for 13 days. The pair now moves together. Doc-guard's
three-warning backlog is cleared with real backfill for M4/M6/M12.

Verified: fmt clean; clippy 5 crates 0 new warnings (74 vs 74 baseline,
counted in a detached worktree at HEAD); lib 2110 passed; cluster_sharding 5;
cluster_runbook 10; tidalctl 38; doc-guard 0 warnings. Playwright 32/34 with
the two remaining failures asserting the rank fix against the not-yet-rolled
image — they are the post-deploy proof.
2026-08-30 20:55:58 -06:00

99 lines
5.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# m4p4 — Session-Aware Ranking and M4 UAT (✅ COMPLETE 2026-02-21)
Phase spec and acceptance criteria: [ROADMAP · Milestone 4 · Phase 4](../ROADMAP.md).
Milestone index: [README.md](README.md). Backfilled record.
## What shipped
1. **`FOR SESSION` on both query surfaces.**
`RetrieveBuilder::for_session(session_id)` and the SEARCH equivalent
(`tidal/src/query/search/executor.rs:265` — `with_session(context, snapshot)`).
The db layer loads the session snapshot and derives a `SessionContext`
(`tidal/src/db/query_ops.rs:172,440`), so an archived session works exactly
like a live one — frozen values instead of decayed-to-now values.
2. **`SessionContext` as the ranking input** (`tidal/src/session/snapshot.rs:54`).
Built by `SessionContext::from_snapshot`: annotation text is split on
whitespace, lowercased, and de-duplicated through a `HashSet` into
`keywords`; `reward_velocity` is the current (or frozen) score of the
`reward` signal; session `metadata` rides along.
3. **The boost formula** (`ProfileExecutor::session_boost`,
`tidal/src/ranking/executor/mod.rs:605-627`):
```
hint_score = matched_keywords / total_keywords // [0,1]
vel_norm = reward_velocity / (reward_velocity+1) // Michaelis-Menten saturation
boost = hint_score * 0.3 + vel_norm * 0.2
```
A keyword matches if any of the candidate's metadata **values** contains it,
case-insensitively. The boost is **additive** and applied after base scoring,
before min-max normalization, so it layers onto personalization instead of
replacing it. Keywords are lowercased once per query
(`lowered_session_keywords`), not once per candidate.
4. **Session state travels back with the results.** `Results.session_snapshot:
Option<SessionSnapshot>` is populated whenever `for_session` is present, so
an agent gets ranked items and its own session state in one round trip.
5. **A ranking reason code.** `ReasonCode::SessionContext`
(`tidal/src/ranking/reason.rs:71,232`) makes a session-influenced result
explainable rather than mysterious.
6. **The M4 UAT** (`tidal/tests/m4_uat.rs`) — 12 `#[test]` functions, one per
scenario step, covering lifecycle, signals, policy accept/reject, annotations,
snapshot, archive, `FOR SESSION` ranking, isolation, and `AgentId` validation.
## Evidence
| Criterion | Proof |
|-----------|-------|
| `FOR SESSION` query returns results and attaches the snapshot | `m4_uat.rs::step8_for_session_ranking_boost` |
| Keyword hints move ranking | `session_durability.rs::hint_keywords_boost_matching_items`; unit test `ranking/executor/mod.rs::session_boost_keyword_match_is_case_insensitive` |
| Empty keyword set is a no-op (no divide-by-zero, no phantom boost) | unit test `ranking/executor/mod.rs::session_boost_empty_keywords_is_noop` |
| Sessions do not leak across each other in ranking inputs | `m4_uat.rs::step9_session_isolation` |
| Archived session usable as query context | `m4_uat.rs::step7_closed_session_snapshot` |
| RETRIEVE and SEARCH behave identically on a swept session | `review_pass2_query_for_session.rs::retrieve_and_search_degrade_identically_on_missing_session` |
## Divergence from the plan
- **Missing session degrades, it does not error — deliberately reversed.** The
ROADMAP criterion reads: "When `for_session` references a non-existent
session, `LumenError::Query("session not found")` returned." That behaviour
shipped and was then **removed on purpose** by the M0M10 review pass 2
(`9728194`): `FOR SESSION <swept-id>` is a well-formed query, and
`CODING_GUIDELINES.md` §6 ("graceful degradation, never failure") says it must
execute without the boost. Before the fix RETRIEVE returned
`Err(SessionNotFound)` while the structurally identical SEARCH degraded — a
surface-specific outage the moment the session sweeper ran. Both surfaces now
degrade. The ROADMAP criterion is stale; the current behaviour is correct.
- **The `< 5 ms` session-context overhead figure is unevidenced.**
`tidal/benches/session.rs` has exactly the right benchmark
(`retrieve_1k_items/{without_session,with_session}`), but no run of it is
recorded in `docs/`. Measurable on demand
(`cargo bench -p tidaldb --bench session`); not currently measured.
## Known dead field — recorded, not silently tolerated
`SessionContext.signaled_entities` is populated by
`SessionContext::from_snapshot` and documented "for entity-level boost", and
`tidal/src/session/audit.rs` says the set "feeds the FOR SESSION entity-level
boost". **No such boost exists.** `session_boost` reads only `keywords` and
`reward_velocity`; a repo-wide search finds `signaled_entities` reached from a
`SessionContext` only in a test fixture that sets it to `HashSet::new()`. The
field on `SessionSnapshot` is genuinely used — but by cross-session preference
aggregation (`tidal/src/db/sessions.rs:547`, m6p4), never by ranking.
Consequences worth stating plainly:
- The set is computed and cloned on every `FOR SESSION` query for no effect.
- `m4_uat.rs::step8_for_session_ranking_boost` is weaker than its comments
suggest. With no annotation set, `hint_score` is 0 and every candidate
receives the same uniform `vel_norm * 0.2`, which cannot reorder anything, so
its assertion (`rank_with <= rank_without`) holds trivially. The keyword half
of the boost *is* genuinely proven — by
`session_durability.rs::hint_keywords_boost_matching_items` and the
case-insensitivity unit test — so the mechanism works; only the
entity-identity claim in step 8's comments is unbacked.
Not fixed here: this record is a planning-history backfill and does not touch
Rust source. Either the entity boost should be implemented (a session that
rewarded entity 5 arguably should rank entity 5 up) or the field and the two doc
comments should go. Both are real changes needing a real owner.