Skip to content

fix(v3.1): Phase 54.5 truth leaks + pin rustc 1.97 - #35

Merged
RichardHightower merged 3 commits into
mainfrom
feature/phase-54.5-truth-leaks
Aug 30, 2026
Merged

fix(v3.1): Phase 54.5 truth leaks + pin rustc 1.97#35
RichardHightower merged 3 commits into
mainfrom
feature/phase-54.5-truth-leaks

Conversation

@RichardHightower

Copy link
Copy Markdown
Contributor

Why

Phase 54's public claims were true. A review of the wiring found second-order honesty bugs in the explainability payload, a hole in LLM rerank (empty event previews + salience undoing the model order), and two landmines that would contaminate Phase 55 measurements. Main is also red on Clippy because CI tracks floating stable (Rust 1.98 result_large_err on generated tonic stubs).

This is the cleanup the review asked for before more measurement.

What changed

CI / toolchain

  • rust-toolchain.toml pins 1.97 so a new stable lint cannot redden main overnight
  • CI / e2e-cli / release use dtolnay/rust-toolchain@master so the pin is honored
  • #![allow(clippy::result_large_err)] on the include_proto! module (we do not control generated Result<_, tonic::Status> signatures)

Explainability tells the truth

  • LLM fail-open (completer error or unparseable order) reports rerank=heuristic
  • layers_attempted only lists layers that were supported and invoked
  • stop_conditions and mode_override are forwarded into query_ranked_with
  • unknown rerank_mode is InvalidArgument; CLI clap-rejects too

LLM rerank actually reranks

  • BM25 text field is now TEXT | STORED (events have empty keywords; the reranker was judging empty bodies)
  • RankingConfig.preserve_order after a successful LLM rerank so salience cannot undo the paid-for order

Don't plant Phase 55 landmines

  • dropped per-event full grip scan on BM25 outbox drain (find_grip_for_event)
  • fusion fan-out is concurrent (join_all) unless mode is Sequential
  • sequential timeout breaks the whole fan-out, not just the inner query loop
  • query path, prune job, and dedup share one HNSW Arc<RwLock<_>>
  • dimension comes from the embedder / EMBEDDING_DIM, not a magic 384

Finding to fix

Theme Fix
Failed LLM rerank still reported rerank=llm Reranker::mode_name() + fell_back flag
layers_attempted listed layers that were never invoked filter by executor.supports
stop_conditions / mode_override echoed, not honored query_ranked_with
Unknown --rerank silently became heuristic clap value_parser + gRPC InvalidArgument
Event hits had empty text_preview store text; searcher returns truncated body
Salience re-sort undid LLM order preserve_order
find_grip_for_event O(n) per outbox entry removed from BM25 drain
Sequential fan-out summed latencies join_all unless Sequential
Query HNSW handle diverged from prune open indexes first; share index_handle()
Hardcoded dimension 384 embedder.info().dimension / EMBEDDING_DIM
Clippy result_large_err on tonic stubs allow on generated module + pin 1.97

Tests (local)

Crate Result
memory-orchestrator 34 passed (incl. parallel fan-out, sequential timeout, fail-open mode_name)
memory-retrieval 78 passed (preserve_order)
memory-search 54 passed (event text preview; text is STORED)
memory-service 125 passed (unknown rerank rejected; LLM fail-open reports heuristic)
memory-indexing 48 passed
memory-daemon 93 passed
memory-cli 63 passed (unknown --rerank rejected)
memory-vector 28 passed
clippy -D warnings green
rustfmt --check green

Existing BM25 indexes created before this change do not store text and need a rebuild for event previews. New indexes get it automatically.

Note on Phase 56 (PR #34)

PR #34 is independently MERGEABLE; its Clippy failure is the same 1.98 result_large_err drift this PR retires. Merge this first, then rebase #34.

Close residual claim/reality leaks in the Phase 54 wiring and stop
floating-stable Clippy from reddening main.

- Explainability reports the reranker that actually ran (LLM fail-open
  is heuristic); layers_attempted lists only supported/invoked layers
- Unknown rerank_mode is InvalidArgument; CLI clap-rejects too
- stop_conditions and mode_override are forwarded into the orchestrator
- BM25 text field is STORED so event hits carry a preview for LLM rerank
- Salience preserve_order after a successful LLM rerank
- Concurrent fan-out (join_all) unless Sequential; sequential timeout
  breaks the whole fan-out, not just the inner loop
- Drop per-event grip scan on BM25 outbox drain
- Query, prune, and dedup share one HNSW handle; dimension from embedder
- rust-toolchain.toml pins 1.97; allow result_large_err on include_proto!
@master requires an explicit toolchain input; it does not read
rust-toolchain.toml. Every job failed with "'toolchain' is a required
input". Keep the toml for local rustup; name the same 1.97 channel in CI.
CI clippy includes e2e-tests with -D warnings. The Phase 55 perf_bench
loop tripped explicit_counter_loop, and two ranking_test literals were
missing the new preserve_order field.
@RichardHightower
RichardHightower merged commit 6fc5cdb into main Aug 30, 2026
21 checks passed
@RichardHightower
RichardHightower deleted the feature/phase-54.5-truth-leaks branch August 30, 2026 20:45
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.

2 participants