feat(rlm): add KnowledgeGraphValidator::from_config() with thesaurus loading + 5 tests Refs #2671

Author: Test User

.docs/design-pr2692-kg-validation.md

@@ -0,0 +1,103 @@
+# Implementation Plan: Extract remaining value from PR #2692
+
+**Status**: Draft
+**Research Doc**: `.docs/research-pr2692-kg-validation.md`
+**Author**: AI Agent
+**Date**: 2026-06-29
+**Estimated Effort**: 30 minutes
+
+## Overview
+
+### Summary
+Add `from_config()` method to `KnowledgeGraphValidator` (deduplicating `build_validator` in rlm.rs and executor/mod.rs), add `#[derive(Debug)]` to `ExecuteResult`, and add 5 tests for `from_config()`. Close PR #2692 as superseded.
+
+### Scope
+**In Scope:**
+- `from_config()` on `KnowledgeGraphValidator`
+- `strictness()` accessor
+- 5 unit tests for `from_config()`
+- `#[derive(Debug)]` on `ExecuteResult`
+
+**Out of Scope:**
+- Restructuring QueryLoop validation (main has a better architecture)
+- Adding `kg_thesaurus_path` config field (main uses `thesaurus` field differently)
+- Noise files from PR (39 files of handoff/review artifacts)
+
+**Avoid At All Cost:**
+- Rewriting existing validation architecture
+- Cherry-picking the PR's competing QueryLoop approach
+- Pulling in .review_tmp/, .handoff/, .beads/, .sessions/ files
+
+## Architecture
+
+### Design Decision
+Main already has a working validation architecture (excutor-based validate_command). The PR's approach (direct validator field in QueryLoop) is incompatible and unnecessary. We extract only the complementary pieces.
+
+### Simplicity Check
+The simplest approach: add `from_config()` to `KnowledgeGraphValidator` as a convenience constructor. Don't change anything else. Don't break existing code.
+
+## File Changes
+
+### Modified Files
+| File | Changes |
+|------|---------|
+| `crates/terraphim_rlm/src/validator.rs` | Add `from_config()`, `strictness()` accessor, 5 tests |
+| `crates/terraphim_rlm/src/query_loop.rs` | Add `#[derive(Debug)]` on `ExecuteResult` |
+
+### Files NOT Touched
+- `rlm.rs` — existing `build_validator` stays (migration deferred)
+- `executor/mod.rs` — existing pattern stays
+- `config.rs` — existing fields stay
+- All noise files — excluded
+
+## API Design
+
+### `KnowledgeGraphValidator::from_config()`
+```rust
+/// Build a validator from RLM config fields.
+///
+/// Eliminates the build_validator() duplication between rlm.rs and
+/// executor/mod.rs. Optionally loads a thesaurus from disk.
+pub fn from_config(
+    strictness: KgStrictness,
+    max_retries: u32,
+    thesaurus_path: Option<&str>,
+) -> Self
+```
+
+### `KnowledgeGraphValidator::strictness()`
+```rust
+/// Return the strictness level this validator is configured with.
+pub fn strictness(&self) -> KgStrictness
+```
+
+## Test Strategy
+
+### Tests to Add (in validator.rs)
+| Test | Purpose |
+|------|---------|
+| `test_from_config_normal_no_path_has_no_thesaurus` | GAP-2: deduplication |
+| `test_from_config_permissive_no_path_passes_silently` | Permissive always passes |
+| `test_from_config_strict_respects_max_retries` | Config propagation |
+| `test_from_config_bad_path_falls_back_gracefully` | GAP-1: no panic on bad path |
+| `test_from_config_valid_thesaurus_json` | GAP-1: real thesaurus loading |
+
+## Implementation Steps
+
+### Step 1: Add `from_config()` and `strictness()` to validator.rs
+**Files:** `crates/terraphim_rlm/src/validator.rs`
+**Description:** Add the two methods after the existing constructors
+**Tests:** Implicit (next step)
+
+### Step 2: Add tests for `from_config()`
+**Files:** `crates/terraphim_rlm/src/validator.rs`
+**Description:** Add 5 unit tests in the `mod tests` block
+**Dependencies:** Step 1
+
+### Step 3: Add `#[derive(Debug)]` to `ExecuteResult`
+**Files:** `crates/terraphim_rlm/src/query_loop.rs`
+**Description:** Add `Debug` derive to the enum
+
+### Step 4: Verify build + close PR
+**Files:** None
+**Description:** `cargo check --workspace`, `cargo test -p terraphim_rlm --lib`, close PR #2692

.docs/research-pr2692-kg-validation.md

@@ -0,0 +1,93 @@
+# Research Document: PR #2692 — Wire KG Validation into QueryLoop Hot Paths
+
+**Status**: Draft
+**Author**: AI Agent
+**Date**: 2026-06-29
+
+## Executive Summary
+
+PR #2692 aimed to wire KG validation into QueryLoop hot paths but was created before the merge sprint. The merge sprint landed multiple PRs (#2902, #2913, #3012, #3033, #3034) that independently implemented most of the PR's goals. The PR is largely superseded. Remaining extractable value: `from_config()` on `KnowledgeGraphValidator`, `#[derive(Debug)]` on `ExecuteResult`, and associated tests.
+
+## Essential Questions Check
+
+| Question | Answer | Evidence |
+|----------|--------|----------|
+| Energising? | Yes | Closes a stale PR and extracts remaining value |
+| Leverages strengths? | Yes | Understanding of already-merged codebase |
+| Meets real need? | Yes | Issue #2671 still closed; PR adds missing `from_config()` deduplication |
+
+**Proceed**: Yes (3/3 YES)
+
+## Problem Statement
+
+### Description
+PR #2692 (47 files, 9566 additions) attempts to wire KG validation but has 39 noise files (handoff artifacts, CI fixes, review dumps) and 8 core RLM files. The core work was independently implemented on main through the merge sprint (19 RLM files changed, 1773 lines added). The PR now has merge conflicts and cannot be merged as-is.
+
+### Impact
+- Wastes review time on noise
+- PR blocks issue #2671 which should be closed
+- `from_config()` deduplication method is missing from main
+
+### Success Criteria
+1. Extract unique remaining value from PR #2692
+2. Apply it cleanly to main
+3. Verify build passes
+4. Close PR #2692 as superseded
+
+## Current State Analysis
+
+### What PR #2692 Changed (merge-base to PR branch)
+| File | Lines | What |
+|------|-------|------|
+| `query_loop.rs` | +214 | Validator field, validation in hot paths, tests |
+| `rlm.rs` | +32/-? | Arc<Validator>, from_config calls |
+| `validator.rs` | +134 | from_config() with thesaurus loading + 5 tests |
+| `executor/mod.rs` | +20 | with_validator() wiring in select_executor |
+| `executor/docker.rs` | +27 | validator field, with_validator(), validate impl |
+| `executor/firecracker.rs` | +32 | Same pattern |
+| `executor/local.rs` | +48 | Same pattern + 2 tests |
+| `config.rs` | +21 | kg_thesaurus_path field, blocks_unknown Normal fix |
+
+### What's Already in Main (merge-base to current main)
+| Feature | Where | How |
+|---------|-------|-----|
+| Executor validate() with KG | executor/{local,docker,firecracker}.rs | `Option<Arc<KnowledgeGraphValidator>>` field |
+| with_validator() | executor/{local,docker}.rs | `pub fn with_validator(mut self, v: Option<Arc<...>>)` |
+| validate_command() in QueryLoop | query_loop.rs | `self.executor.validate(input).await` with retry logic |
+| validation_retries cell | query_loop.rs | `Cell<u32>` for retry tracking |
+| blocks_unknown Normal fix | config.rs | Already returns true for Normal mode |
+| Arc<KnowledgeGraphValidator> in rlm | rlm.rs | Already present |
+
+### What's NOT Yet in Main (extractable from PR)
+| Feature | Value |
+|---------|-------|
+| `from_config()` on KnowledgeGraphValidator | Eliminates duplicate build_validator in rlm.rs and executor/mod.rs |
+| `#[derive(Debug)]` on ExecuteResult | Minor but useful for debugging |
+| 5 from_config tests in validator.rs | Needed test coverage |
+| `strictness()` accessor | Exposes validator strictness |
+
+## Constraints
+
+### Technical Constraints
+- Must not break existing validation architecture (executor-based validate_command)
+- Must compile with `cargo check --workspace`
+- Must pass `cargo clippy --workspace`
+- Must pass `cargo fmt --all -- --check`
+
+## Vital Few
+
+| Constraint | Why Vital | Evidence |
+|------------|-----------|----------|
+| Only add from_config, don't restructure | Existing architecture works; PR's approach is incompatible | Main already has 1773 lines of validated RLM changes |
+| Don't pull noise files | 39 files are review artifacts, not code | File list shows .review_tmp/, .handoff/, .beads/, .sessions/ |
+
+## Risks
+
+| Risk | Likelihood | Impact | Mitigation |
+|------|------------|--------|------------|
+| `from_config()` signature mismatch with main | Medium | Low | Check existing validator API before implementing |
+| Tests fail due to different test framework | Low | Medium | Run tests before commit |
+
+## Recommendations
+
+**Proceed** with minimal extraction: add `from_config()` (+ tests) and `#[derive(Debug)]` to main, close PR #2692.

crates/terraphim_rlm/src/validator.rs

@@ -469,6 +469,68 @@ impl KnowledgeGraphValidator {
     pub fn has_role_graph(&self) -> bool {
         self.role_graph.is_some()
     }
+
+    /// Return the strictness level this validator is configured with.
+    pub fn strictness(&self) -> KgStrictness {
+        self.config.strictness
+    }
+
+    /// Build a validator from RLM config fields.
+    ///
+    /// Eliminates the `build_validator()` duplication between rlm.rs and
+    /// executor/mod.rs (GAP-2).  Optionally loads a thesaurus from disk
+    /// so Normal/Strict validation performs real term matching (GAP-1).
+    pub fn from_config(
+        strictness: KgStrictness,
+        max_retries: u32,
+        thesaurus_path: Option<&str>,
+    ) -> Self {
+        let mut vcfg = match strictness {
+            KgStrictness::Permissive => ValidatorConfig::permissive(),
+            KgStrictness::Normal => ValidatorConfig::default(),
+            KgStrictness::Strict => ValidatorConfig::strict(),
+        };
+        vcfg.max_retries = max_retries;
+        let mut validator = Self::new(vcfg);
+
+        match thesaurus_path {
+            Some(path) => match std::fs::read_to_string(path) {
+                Ok(json) => match serde_json::from_str::<Thesaurus>(&json) {
+                    Ok(thesaurus) => {
+                        log::info!(
+                            "KG validator: loaded {} terms from {}",
+                            thesaurus.len(),
+                            path
+                        );
+                        validator = validator.with_thesaurus(thesaurus);
+                    }
+                    Err(e) => log::warn!(
+                        "KG validator: could not parse thesaurus at '{}': {}. \
+                         Real term matching is disabled.",
+                        path,
+                        e
+                    ),
+                },
+                Err(e) => log::warn!(
+                    "KG validator: could not read thesaurus at '{}': {}. \
+                     Real term matching is disabled.",
+                    path,
+                    e
+                ),
+            },
+            None if strictness != KgStrictness::Permissive => {
+                log::warn!(
+                    "KG validator: no thesaurus configured (thesaurus_path is None). \
+                     {:?} mode will not perform real term matching. \
+                     Set thesaurus_path to a JSON thesaurus file to enable.",
+                    strictness
+                );
+            }
+            None => {}
+        }
+
+        validator
+    }
 }
 
 /// Extract words from a command string.
@@ -766,4 +828,59 @@ mod tests {
         );
         assert!(!result.matched_terms.is_empty());
     }
+
+    // -- from_config tests (GAP-2: deduplication; GAP-1: thesaurus loading) --
+
+    #[test]
+    fn test_from_config_normal_no_path_has_no_thesaurus() {
+        let v = KnowledgeGraphValidator::from_config(KgStrictness::Normal, 3, None);
+        assert!(!v.has_thesaurus());
+        assert_eq!(v.strictness(), KgStrictness::Normal);
+        assert_eq!(v.config().max_retries, 3);
+    }
+
+    #[test]
+    fn test_from_config_permissive_no_path_passes_silently() {
+        let v = KnowledgeGraphValidator::from_config(KgStrictness::Permissive, 0, None);
+        assert!(!v.has_thesaurus());
+        let result = v.validate("any command here").unwrap();
+        assert!(result.passed);
+    }
+
+    #[test]
+    fn test_from_config_strict_respects_max_retries() {
+        let v = KnowledgeGraphValidator::from_config(KgStrictness::Strict, 5, None);
+        assert_eq!(v.config().max_retries, 5);
+        assert_eq!(v.strictness(), KgStrictness::Strict);
+    }
+
+    #[test]
+    fn test_from_config_bad_path_falls_back_gracefully() {
+        let v = KnowledgeGraphValidator::from_config(
+            KgStrictness::Normal,
+            3,
+            Some("/nonexistent/path/thesaurus.json"),
+        );
+        assert!(!v.has_thesaurus());
+        let result = v.validate("some command").unwrap();
+        assert!(result.passed);
+    }
+
+    #[test]
+    fn test_from_config_valid_thesaurus_json() {
+        use std::io::Write;
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("thesaurus.json");
+        let thesaurus_json = r#"{"name":"test","data":{}}"#;
+        let mut f = std::fs::File::create(&path).unwrap();
+        f.write_all(thesaurus_json.as_bytes()).unwrap();
+        drop(f);
+
+        let v = KnowledgeGraphValidator::from_config(
+            KgStrictness::Normal,
+            3,
+            Some(path.to_str().unwrap()),
+        );
+        assert!(v.has_thesaurus());
+    }
 }