-
Notifications
You must be signed in to change notification settings - Fork 3
feat: knowledge graph validation workflows for pre/post-LLM #383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…mentation Previously the MCP tool was a placeholder that only found matched terms without actually checking graph connectivity. Now it: - Gets RoleGraphSync directly from config_state.roles - Calls the real RoleGraph::is_all_terms_connected_by_path() method - Returns detailed connectivity results with matched term names - Provides semantic interpretation (connected = coherent, not connected = unrelated) Also updates tests to use correct "text" parameter instead of "terms" array. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add two new CLI subcommands for knowledge graph validation: - `validate --connectivity`: Check if matched terms are connected by a single path in the knowledge graph. Useful for pre-LLM semantic coherence validation. - `suggest --fuzzy`: Fuzzy autocomplete suggestions when exact matches aren't found. Uses Jaro-Winkler similarity with configurable threshold. Both commands support: - `--role` flag for role-specific knowledge graph - `--json` flag for machine-readable output - stdin input when text/query not provided Part of Phase B implementing local-first KG validation workflows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add checklist validation mode to the validate command: - `validate --checklist code_review "text"` - validates against code review checklist (tests, documentation, error handling, security, performance) - `validate --checklist security "text"` - validates against security checklist (authentication, authorization, input validation, encryption, logging) Checklist definitions stored in docs/src/kg/checklists/ for future dynamic loading from knowledge graph files. Usage: terraphim-agent validate --checklist code_review "Added tests and docs" terraphim-agent validate --checklist security --json "Auth with SSL" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add new `hook` command that provides a single entry point for all
Claude Code hook types:
- `pre-tool-use`: Intercepts Bash commands for KG-based replacement
- `post-tool-use`: Validates tool output via connectivity check
- `pre-commit`/`prepare-commit-msg`: Extracts concepts from diff
Usage:
echo '{"tool_name":"Bash","tool_input":{"command":"npm install"}}' | \
terraphim-agent hook --hook-type pre-tool-use
All hooks output JSON for seamless Claude Code integration.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add complete skill and hook infrastructure for knowledge graph validation workflows: Skills: - pre-llm-validate: Validate input before LLM calls - post-llm-check: Validate outputs against domain checklists - smart-commit: Enhance commits with extracted concepts Hooks: - pre-llm-validate.sh: PreToolUse hook for semantic validation - post-llm-check.sh: PostToolUse hook for checklist validation - prepare-commit-msg: Updated with optional concept extraction Enable smart commit with: TERRAPHIM_SMART_COMMIT=1 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update project documentation with new knowledge graph validation features: CLAUDE.md: - Added pre-llm-validate and post-llm-check hooks - Documented validate/suggest/hook CLI commands - Added smart commit usage examples install-terraphim-hooks.sh: - Install all new hooks (pre-llm, post-llm) - Show complete feature list - Updated usage examples lessons-learned.md: - MCP placeholder detection pattern - Checklist as KG concept pattern - Unified hook handler pattern - Role-aware validation pattern - CLI with JSON output pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update project documentation with new knowledge graph validation features: CLAUDE.md: - Added pre-llm-validate and post-llm-check hooks - Documented validate/suggest/hook CLI commands - Added smart commit usage examples install-terraphim-hooks.sh: - Install all new hooks (pre-llm, post-llm) - Show complete feature list - Updated usage examples lessons-learned.md: - MCP placeholder detection pattern - Checklist as KG concept pattern - Unified hook handler pattern - Role-aware validation pattern - CLI with JSON output pattern 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…rkflows Comprehensive handover document covering: - All 5 implementation phases (A-E) - 7 commits on architecture-review branch - Testing verification and usage examples - Next steps and future enhancements - Technical deep dive on key components Ready for PR creation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
GitHub Runner Execution ResultsPR: #383 - feat: knowledge graph validation workflows for pre/post-LLM ❌ ci.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ test-firecracker-runner.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ test-on-pr.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ vm-execution-tests.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ ci-native.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ✅ Powered by terraphim-github-runner |
Add disciplined development artifacts: - Phase 1 research on underutilized features - Phase 2 design plan with step-by-step sequence - Session log tracking implementation progress 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
GitHub Runner Execution ResultsPR: #383 - feat: knowledge graph validation workflows for pre/post-LLM ❌ ci.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ test-on-pr.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ test-firecracker-runner.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ vm-execution-tests.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ❌ ci-native.ymlExecution failed: VM allocation failed: Allocation failed with status: 429 Too Many Requests ✅ Powered by terraphim-github-runner |
Code Review: Knowledge Graph Validation WorkflowsThis PR implements a comprehensive local-first knowledge graph validation system. Overall excellent architecture, but critical security issues must be addressed before merge. Critical Issues (Must Fix Before Merge)1. Shell Command Injection Risk Problem: PROMPT and TOOL_RESULT passed directly to shell without escaping. Shell metacharacters could execute arbitrary commands. Fix: Use stdin instead of command-line arguments for untrusted input. 2. Missing Input Validation Problem: No size limits on stdin reads - vulnerable to memory exhaustion. Fix: Add MAX_INPUT_SIZE constant (1MB) and use take() to limit reads. High Priority Issues3. Missing Test Coverage 4. Missing Rustdoc Examples 5. Magic Numbers Strengths
Performance Notes
VerdictConditional Approval ✅ (with required fixes) The shell injection vulnerability must be fixed before merge. Other issues can be addressed in follow-up PRs. Great work on the local-first validation approach! Estimated fix time: 2-3 hours for critical issues |
|
Detailed Fix Examples Fix 1 Shell Injection Prevention Current vulnerable code in .claude/hooks/pre-llm-validate.sh line 60: Fixed version - pass untrusted input via stdin not command-line args: Apply same fix to post-llm-check.sh line 57. Fix 2 Input Size Validation Add MAX_INPUT_SIZE constant and use take() to limit stdin reads. This prevents memory exhaustion attacks. Fix 3 Extract Magic Numbers Add constants to main.rs:
Fix 4 Add Checklist Validation Tests Create test_checklist_validation() with scenarios for:
These fixes address the critical security issues and improve code quality. Priority: Fix 1 and 2 before merge. |
Code Review: Knowledge Graph Validation WorkflowsThis PR implements comprehensive knowledge graph validation for pre/post-LLM workflows. Overall excellent architectural work that effectively leverages underutilized Terraphim features. ✅ StrengthsArchitecture & Design
Code Quality
Testing & Documentation
🔍 Critical Issues1. Hook Files Incomplete
|
Code Review: Knowledge Graph Validation WorkflowsThis is a well-structured feature addition that leverages existing Terraphim capabilities. StrengthsArchitecture & Design
Code Quality
Implementation Highlights
Issues & Concerns1. Security: Shell Command Injection Risk (Medium Priority) The current implementation passes user input directly to shell commands. If input contains special characters, this could be exploited. Use safer argument passing or stdin-based input with proper escaping. 2. Performance: No Timeout Enforcement (Medium Priority) 3. Error Handling: Silent Failures (Low Priority) 4. Code Duplication: Hardcoded Checklists (Medium Priority) Checklists are hardcoded despite having markdown files in docs/src/kg/checklists/. Parse checklist markdown files at runtime using existing KG infrastructure. 5. Missing: Term Limit Enforcement (Low Priority) 6. Bug: Case Sensitivity Mismatch Checklist validation uses .to_lowercase() string comparison instead of the autocomplete index case-insensitive matching. Testing ObservationsGood:
Missing:
Recommendations by PriorityHigh Priority (Before Merge)
Medium Priority (Follow-up PR) Low Priority (Future Enhancement) Security ConsiderationsOWASP Relevance: A1 (Injection), A5 (Security Misconfiguration) Concerns:
Final VerdictRecommendation: Approve with required changes This is a valuable feature that unlocks underutilized Terraphim capabilities. The architecture is sound, the implementation is mostly clean, and the testing is adequate for initial release. Required changes before merge:
Suggested follow-up PR:
The feature demonstrates excellent understanding of the codebase and provides clear value for AI coding workflows. With the security fixes applied, this will be a strong addition to Terraphim. Great work on this implementation! The disciplined methodology clearly paid off in the quality of the result. |
Documentation PreviewYour documentation changes have been deployed to: This preview will be available until the PR is closed. |
Documentation PreviewYour documentation changes have been deployed to: This preview will be available until the PR is closed. |
Code Review: Knowledge Graph Validation WorkflowsI've completed a comprehensive review of this PR. Overall, this is a high-quality implementation with excellent documentation. ✅ Key Strengths
🔍 Critical Issues (Must Fix)1. Security: Shell Injection Risk
|
Summary
Implements local-first knowledge graph validation workflows using underutilized Terraphim features. Enables pre-LLM semantic validation and post-LLM domain compliance checking.
Implementation Phases
Phase A: Foundation
is_all_terms_connected_by_pathplaceholder to call real RoleGraph implementationtextparameterPhase B: CLI Commands
validate --connectivity- Check semantic coherence via graph pathsvalidate --checklist- Validate against domain checklists (code_review, security)suggest --fuzzy- Fuzzy autocomplete with Jaro-Winkler similarityhook --hook-type- Unified handler for all Claude Code hooksPhase C: Skills & Hooks
Phase D: Knowledge Graph Extensions
docs/src/kg/checklists/Phase E: Integration & Documentation
New Capabilities
CLI Commands:
```bash
Validate semantic connectivity
terraphim-agent validate --connectivity "haystack service automata"
Validate against code review checklist
terraphim-agent validate --checklist code_review "Added tests and docs"
Fuzzy suggestions for typos
terraphim-agent suggest "terraphm" --threshold 0.7
Unified hook handler
terraphim-agent hook --hook-type pre-tool-use --input "$JSON"
```
Skills:
skills/pre-llm-validate/- Pre-LLM semantic validation guideskills/post-llm-check/- Post-LLM checklist validation guideskills/smart-commit/- Commit message enrichment guideEnable Smart Commit:
```bash
export TERRAPHIM_SMART_COMMIT=1
git commit -m "feat: your feature"
Commit enriched with concepts from diff
```
Test Plan
Automated Tests
Manual Verification
validate --connectivityreturns correct true/false resultsvalidate --checklistidentifies satisfied/missing items correctlysuggest --fuzzyreturns ranked suggestions with similarity scoreshook --hook-type pre-tool-useintercepts and replaces commandsIntegration Tests (Manual)
```bash
Test connectivity
./target/debug/terraphim-agent validate --connectivity --json "haystack service"
Expected: {"connected":false,"matched_terms":["haystack","service"],...}
Test checklist (all satisfied)
./target/debug/terraphim-agent validate --checklist code_review --json
"Code includes tests, docs, error handling, security checks, and performance optimization"
Expected: {"passed":true,"total_items":5,"satisfied":[all],"missing":[]}
Test checklist (partial)
./target/debug/terraphim-agent validate --checklist code_review "Added tests and docs"
Expected: {"passed":false,"satisfied":["tests","error_handling"],"missing":[...]}
Test fuzzy suggest
./target/debug/terraphim-agent suggest "terraphm" --threshold 0.7
Expected: terraphim-graph (similarity: 75.43), ...
Test hook
echo '{"tool_name":"Bash","tool_input":{"command":"npm install"}}' |
./target/debug/terraphim-agent hook --hook-type pre-tool-use
Expected: Modified JSON with "bun install"
```
Documentation
HANDOVER.mdfor complete handover.sessions/implementation-summary.mdfor detailed summarylessons-learned.mdfor 5 new patterns learnedBreaking Changes
None - all changes are additive and backward compatible.
Performance
🤖 Generated with Claude Code
Co-Authored-By: Claude Sonnet 4.5 (1M context) noreply@anthropic.com