Fix #1990: wire suggest_learnings (BM25) into pre-tool-use hook by AlexMikhalev · Pull Request #904 · terraphim/terraphim-ai

AlexMikhalev · 2026-06-04T16:06:58Z

Problem

crates/terraphim_agent/src/learnings/capture.rs had four symbols marked #[allow(dead_code)] and never called from any live code path:

Symbol	Type
`ScoredEntry` struct	Wraps `LearningEntry` with relevance score
`TranscriptEntry` struct	Parsed JSONL transcript entries
`extract_transcript_entries()` fn	Extracted `TranscriptEntry` objects from JSONL
`suggest_learnings()` fn	BM25-ranked learning suggestions

Fix

Option A (recommended in issue): Wire suggest_learnings into process_pre_tool_use.

Changes:

hook.rs: Replace query_learnings (simple text match by base command) with suggest_learnings (BM25-style keyword relevance scoring against the full command string). Results are ordered by relevance — most contextually similar failure surfaces first.
capture.rs: Delete TranscriptEntry, contains_correction_phrase, extract_command_from_input, and auto_extract_corrections — all were dead code unreachable from any live path. Remove all #[allow(dead_code)] annotations from ScoredEntry, score_entry_relevance, and suggest_learnings which are now active.
capture.rs: Add unit tests for the newly-wired suggest_learnings function covering empty storage, keyword matching, short-keyword fallback, limit parameter, and result ordering.

Acceptance Criteria

suggest_learnings wired into process_pre_tool_use
No #[allow(dead_code)] annotations remain on these symbols
cargo clippy -p terraphim_agent -- -D warnings passes clean
242 unit tests pass (cargo test -p terraphim_agent --lib)

Verification

cargo clippy -p terraphim_agent -- -D warnings # clean
cargo test -p terraphim_agent --lib # 242 passed, 0 failed

Refs terraphim/terraphim-ai#1990 (Gitea)

…1992 reqwest::Error from error_for_status() on a 400 response is an HTTP application error, not a transport/connectivity failure. Previously classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest errors, causing test_server_mode_search_with_selected_role to fail when the server returned 400 due to missing role configuration. Now check re.status() before falling back to ErrorNetwork: - 401 / 403 → ErrorAuth (5) - 404 → ErrorNotFound (4) - other 4xx/5xx → ErrorGeneral (1) - no status → ErrorNetwork (6, true connectivity failure) Adds five regression tests using a real one-shot TCP server so no mocks are needed, covering 400, 401, 403, 404, 500 paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds an integration test that starts a real TCP listener replying with HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6). This exercises the classify_error fix end-to-end through the real binary, complementing the unit-level classify_reqwest_tests in main.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…_node_ids Refs #2039 Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based KG retrieval) was merged would fail with a missing-field error because trigger_descriptions and pinned_node_ids had no serde(default) annotation. Adds the annotation to both fields and a round-trip regression test that strips the fields from a serialised graph and confirms deserialisation succeeds with empty collections, matching the existing learning_document_ids pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…2133 Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with `for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…(feature = "firecracker") firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally. This broke cargo test -p terraphim_agent with default features. Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs and update HybridExecutor to conditionally use FirecrackerExecutor only when the feature is enabled, falling back to LocalExecutor otherwise. Refs #2164 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- haystack_atlassian: add /// to ConfluenceClient, JiraClient, AtlassianClient - haystack_core: add /// to HaystackProvider trait and associated items - haystack_discourse: add /// to DiscourseClient and Post re-exports - terraphim_ccusage: add /// to all public types and CcusageClient methods - terraphim_kg_linter: add /// to all public structs, enums, and functions - terraphim_negative_contribution: add /// to re-exported public items - CHANGELOG: add firecracker gate, clippy manual_flatten, rolegraph serde defaults, and rustdoc coverage improvement entries Workspace doc coverage: 81% (was 30% at prior scan). Zero-coverage crates: 1. Refs #2136

…ubmodules - haystack_discourse: add /// to DiscourseClient struct, new(), Post struct - terraphim_negative_contribution: add /// to NegativeContributionScanner and all pub methods (new, from_thesaurus, scan_file, scan_files, scan_to_output, thesaurus) Reduces zero-coverage crate count to 0. Refs #2136

… #1990 Replace query_learnings (simple text match) with suggest_learnings (BM25-style keyword scoring) in the pre-tool-use hook. Results are ordered by relevance so the most contextually similar failure is surfaced first. Delete TranscriptEntry, contains_correction_phrase, extract_command_from_input, and auto_extract_corrections along with their test coverage — all were dead code unreachable from any live path. Remove #[allow(dead_code)] from ScoredEntry, score_entry_relevance, and suggest_learnings which are now active call sites. Acceptance criteria: - suggest_learnings wired into process_pre_tool_use (Option A) - No #[allow(dead_code)] on the four target symbols - cargo clippy -p terraphim_agent -- -D warnings passes - 45 capture + 6 hook unit tests pass

Test coverage for the newly-wired suggest_learnings function: - empty storage returns empty vec - matching context keywords returns scored entries - short keywords (<=2 chars) fall back to recent entries - limit parameter is respected - results are ordered by score descending

AlexMikhalev · 2026-08-29T23:24:01Z

Closing as superseded. This PR is a Fix #1990: wire suggest_learni... patch based on an older state of main. The substantive intent has been overtaken by the polyrepo fleet-batch + the recent rebase wave (17+ PRs merged 2026-08-28/29). A rebase would require resolving hundreds of conflicts against substantial refactors. Per the user instruction "fully functional and green," these stale PRs are being closed rather than re-rebased.

Closes per Shimaguru mass-rebase pass, 2026-08-29.

AlexMikhalev · 2026-06-04T16:06:58Z