* feat: implement hierarchical configuration (system, tenant, bank) * feat: implement hierarchical configuration (system, tenant, bank) * docs: add instructions for hierarchical config in CLAUDE.md * feat: add ENABLE_BANK_CONFIG_API flag (disabled by default) - Add HINDSIGHT_API_ENABLE_BANK_CONFIG_API env var (default: false) - Return 403 Forbidden from bank config endpoints when disabled - Update tests to enable the flag - Update CLAUDE.md documentation This provides security control over the bank configuration API, ensuring it's only accessible when explicitly enabled. * docs: add hierarchical configuration section * feat(cli): add bank config commands (config, set-config, reset-config) - Add 'hindsight bank config' to view bank configuration - Add 'hindsight bank set-config' to update LLM settings per bank - Add 'hindsight bank reset-config' to reset to defaults - Implements client API calls to new bank config endpoints * fix(cli): fix compilation errors in bank config commands - Fix type signature: use ApiClient instead of api::Client - Fix confirmation: use ui::prompt_confirmation instead of ui::confirm - Fix error handling: use anyhow! macro instead of errors::Error - Fix type conversion: convert HashMap to serde_json::Map for API call * feat: implement type-safe hierarchical config with bank overrides Implements a production-ready hierarchical configuration system that prevents accidentally using global defaults when bank-specific overrides exist. - Created StaticConfigProxy that wraps HindsightConfig - get_config() now returns proxy that blocks access to bank-configurable fields - Raises ConfigFieldAccessError with clear message when accessing configurable fields - Added _get_raw_config() for internal use only - Forces developers to use resolve_full_config(bank_id, context) for bank settings - Added resolve_full_config() method that returns complete HindsightConfig - Resolves hierarchy: Global (env) → Tenant → Bank - No caching to support multi-server deployments (always fresh from DB) - LLM provider pooling handles expensive operations separately - Updated entire retain pipeline to pass resolved config through call chain - memory_engine.py: Resolves config at top level where bank_id/context available - orchestrator.py: Accepts and passes config to fact_extraction - fact_extraction.py: Uses passed config instead of get_config() - utils.py: Added optional config param for backward compatibility - consolidator.py: Uses resolve_full_config() for enable_observations check - memory_engine.py: Resolves config before triggering consolidation - Renamed "Memory Bank" to "Bank Configuration" with tabs - Combined Stats and Operations into "General" tab - Consolidated Profile and Configuration into "Configuration" tab - Moved Actions dropdown to page level (outside tabs) - Created new component for managing bank-specific config - Displays configurable fields: retain_chunk_size, retain_extraction_mode, etc. - Edit via dialog with form validation - Reset to defaults via AlertDialog confirmation - Shows field IDs in monospace for clarity - Visual separation with borders and hover effects - Removed inline edit mode, switched to dialog-based editing - Separate dialogs for Disposition and Mission editing - Read-only display with clear edit buttons - Removed duplicate stats cards and operations - bank-stats-view.tsx: Overview statistics (memories, links, documents, pending ops) - bank-operations-view.tsx: Background operations table with filtering **Problem**: Consolidation always used global enable_observations, ignoring bank overrides **Root Cause**: consolidator.py called get_config() instead of resolving bank-specific config **Solution**: Pass resolved config through the entire pipeline **Problem**: asyncpg returning JSONB as JSON string instead of parsed dict **Solution**: Explicit JSON parsing in config_resolver.py with type checking - All 19 API integration tests pass - All 10 hierarchical config tests pass - Retain operations work correctly with bank-specific config - Consolidation respects bank-specific enable_observations setting - Updated developer/configuration.md with type-safe config access pattern - Added examples showing correct usage patterns - Documented ConfigFieldAccessError and resolution methods - get_config() now returns StaticConfigProxy (blocks configurable field access) - Code accessing bank-configurable fields must use resolve_full_config() - Clear migration path with helpful error messages Fixes hierarchical configuration to be production-ready with proper type safety. * refactor: remove LLM client pool and simplify config resolver Since LLM config (provider, model, api_key) is now static and not bank-configurable, the LLMClientPool is no longer needed. Changes: - Remove hindsight_api/llm_client_pool.py (no longer needed) - Remove memory_engine._get_bank_llm_config() (dead code, never called) - Simplify config_resolver.py by eliminating duplication between resolve_full_config() and get_bank_config() - get_bank_config() now calls resolve_full_config() and filters results - Remove outdated "LLM provider pooling" comments from docstrings All tests pass (10 hierarchical config tests, 19 API integration tests) * fix: update tests to use _get_raw_config() for configurable fields Fixed test fixtures that were accessing configurable fields (like enable_observations) from get_config(), which now raises ConfigFieldAccessError due to type-safe config access. Changes: - test_consolidation.py: Changed enable_observations fixture to use _get_raw_config() instead of get_config() - test_consolidation.py: Updated test_consolidation_returns_disabled_status to set bank config instead of mocking get_config() - test_link_expansion_retrieval.py: Changed fixture to use _get_raw_config() - test_observations.py: Changed disable_observations fixture to use _get_raw_config() - Regenerated OpenAPI spec and clients All 39 previously failing tests now pass. * fix: add missing config parameter to test calls of extract_facts_from_text() Fixed 45 test failures where tests were calling extract_facts_from_text() without the new required config parameter. Changes: - Added config=_get_raw_config() to all extract_facts_from_text() calls - Fixed test_main_module.py to patch _get_raw_config instead of get_config - Updated 6 test files with 37 function call sites All tests should now pass. * fix: add missing config parameter to test_skip_podcast_meta_commentary One more test was missing the config parameter for extract_facts_from_text().
103 lines
3.5 KiB
Python
103 lines
3.5 KiB
Python
"""
|
|
Test to analyze fact extraction token usage and identify optimization opportunities.
|
|
"""
|
|
import asyncio
|
|
import logging
|
|
import time
|
|
from datetime import datetime
|
|
|
|
import pytest
|
|
|
|
from hindsight_api.config import get_config, clear_config_cache, _get_raw_config
|
|
from hindsight_api.engine.llm_wrapper import LLMConfig
|
|
from hindsight_api.engine.retain.fact_extraction import extract_facts_from_text
|
|
|
|
logging.basicConfig(level=logging.INFO)
|
|
logger = logging.getLogger(__name__)
|
|
|
|
|
|
@pytest.fixture
|
|
def llm_config():
|
|
"""Create LLM config from environment."""
|
|
clear_config_cache()
|
|
config = get_config()
|
|
return LLMConfig(
|
|
provider=config.retain_llm_provider or config.llm_provider,
|
|
api_key=config.retain_llm_api_key or config.llm_api_key,
|
|
model=config.retain_llm_model or config.llm_model,
|
|
base_url=config.retain_llm_base_url or config.llm_base_url,
|
|
)
|
|
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_fact_extraction_basic_analysis(llm_config):
|
|
"""
|
|
Test fact extraction and analyze token usage with sample content.
|
|
|
|
This test helps identify:
|
|
1. How many facts are extracted
|
|
2. Token usage (input/output ratio)
|
|
3. Types of facts being extracted
|
|
"""
|
|
content = """
|
|
Alice is a senior software engineer at TechCorp with 8 years of experience.
|
|
She has a Kubernetes certification (CKA) and leads the platform team.
|
|
Bob is her colleague who works on the frontend. He's been at the company for 3 years.
|
|
They're working on a new microservices migration project together.
|
|
The deadline for the first milestone is end of Q2.
|
|
Alice prefers to use Go for backend services while Bob advocates for TypeScript.
|
|
"""
|
|
|
|
logger.info(f"Content length: {len(content)} chars (~{len(content) // 4} tokens)")
|
|
|
|
start_time = time.time()
|
|
|
|
facts, chunks, usage = await extract_facts_from_text(
|
|
text=content,
|
|
event_date=datetime.now(),
|
|
llm_config=llm_config,
|
|
agent_name="test-agent",
|
|
context="Friday Standup meeting",
|
|
config=_get_raw_config(),
|
|
)
|
|
|
|
duration = time.time() - start_time
|
|
|
|
logger.info(f"\n{'='*60}")
|
|
logger.info(f"EXTRACTION RESULTS")
|
|
logger.info(f"{'='*60}")
|
|
logger.info(f"Duration: {duration:.2f}s")
|
|
logger.info(f"Chunks: {len(chunks)}")
|
|
logger.info(f"Facts extracted: {len(facts)}")
|
|
logger.info(f"Input tokens: {usage.input_tokens}")
|
|
logger.info(f"Output tokens: {usage.output_tokens}")
|
|
logger.info(f"Token ratio (out/in): {usage.output_tokens / max(1, usage.input_tokens):.2f}")
|
|
|
|
# Analyze facts by type
|
|
fact_types = {}
|
|
for fact in facts:
|
|
ft = fact.fact_type
|
|
fact_types[ft] = fact_types.get(ft, 0) + 1
|
|
|
|
logger.info(f"\nFacts by type:")
|
|
for ft, count in sorted(fact_types.items()):
|
|
logger.info(f" {ft}: {count}")
|
|
|
|
# Show sample facts
|
|
logger.info(f"\nSample facts (first 10):")
|
|
for i, fact in enumerate(facts[:10]):
|
|
logger.info(f"\n [{i+1}] {fact.fact_type}: {fact.fact[:150]}...")
|
|
|
|
# Show facts containing key terms
|
|
key_terms = ["kubernetes", "k8s", "CKA", "certification", "Alice"]
|
|
logger.info(f"\n{'='*60}")
|
|
logger.info(f"FACTS CONTAINING KEY TERMS")
|
|
logger.info(f"{'='*60}")
|
|
|
|
for term in key_terms:
|
|
matching = [f for f in facts if term.lower() in f.fact.lower()]
|
|
logger.info(f"\n'{term}' ({len(matching)} facts):")
|
|
for fact in matching[:3]:
|
|
logger.info(f" - {fact.fact[:200]}...")
|
|
|
|
assert len(facts) > 0, "Should extract at least one fact"
|