Fix #1990: wire suggest_learnings (BM25) into pre-tool-use hook#904
Open
AlexMikhalev wants to merge 13 commits into
Open
Fix #1990: wire suggest_learnings (BM25) into pre-tool-use hook#904AlexMikhalev wants to merge 13 commits into
AlexMikhalev wants to merge 13 commits into
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
crates/terraphim_agent/src/learnings/capture.rshad four symbols marked#[allow(dead_code)]and never called from any live code path:ScoredEntrystructLearningEntrywith relevance scoreTranscriptEntrystructextract_transcript_entries()fnTranscriptEntryobjects from JSONLsuggest_learnings()fnFix
Option A (recommended in issue): Wire
suggest_learningsintoprocess_pre_tool_use.Changes:
hook.rs: Replacequery_learnings(simple text match by base command) withsuggest_learnings(BM25-style keyword relevance scoring against the full command string). Results are ordered by relevance — most contextually similar failure surfaces first.capture.rs: DeleteTranscriptEntry,contains_correction_phrase,extract_command_from_input, andauto_extract_corrections— all were dead code unreachable from any live path. Remove all#[allow(dead_code)]annotations fromScoredEntry,score_entry_relevance, andsuggest_learningswhich are now active.capture.rs: Add unit tests for the newly-wiredsuggest_learningsfunction covering empty storage, keyword matching, short-keyword fallback, limit parameter, and result ordering.Acceptance Criteria
suggest_learningswired intoprocess_pre_tool_use#[allow(dead_code)]annotations remain on these symbolscargo clippy -p terraphim_agent -- -D warningspasses cleancargo test -p terraphim_agent --lib)Verification
Refs terraphim/terraphim-ai#1990 (Gitea)