* 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().
401 lines
16 KiB
Python
401 lines
16 KiB
Python
"""
|
|
Tests for hindsight_api.main module (single-worker code path).
|
|
|
|
The main.py module is used when running with a single worker:
|
|
hindsight-api (or hindsight-api --workers 1)
|
|
|
|
When workers=1, main.py creates the app directly and passes it to uvicorn.
|
|
These tests ensure that extensions are properly loaded in this code path.
|
|
|
|
Compare with test_server_module.py which tests the multi-worker path (workers > 1).
|
|
"""
|
|
|
|
import sys
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
|
|
class TestMainModuleExtensionLoading:
|
|
"""Tests that main.py correctly loads extensions when configured via environment."""
|
|
|
|
def test_main_loads_tenant_extension_when_configured(self, monkeypatch):
|
|
"""
|
|
Verify that main.py loads tenant extension from HINDSIGHT_API_TENANT_EXTENSION.
|
|
|
|
This ensures extension loading works in the single-worker code path.
|
|
"""
|
|
# Set up environment to configure a tenant extension
|
|
monkeypatch.setenv(
|
|
"HINDSIGHT_API_TENANT_EXTENSION",
|
|
"tests.test_main_module:MockTenantExtension",
|
|
)
|
|
# Ensure single worker mode
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
|
|
# Track what extensions were loaded via load_extension
|
|
loaded_extensions = {}
|
|
|
|
# Get the real load_extension function
|
|
from hindsight_api.extensions.loader import load_extension as real_load_extension
|
|
|
|
def tracking_load_extension(name, base_class):
|
|
"""Track calls to load_extension and delegate to original."""
|
|
result = real_load_extension(name, base_class)
|
|
loaded_extensions[name] = result
|
|
return result
|
|
|
|
with patch("hindsight_api.main.MemoryEngine") as mock_engine, \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.load_extension", side_effect=tracking_load_extension), \
|
|
patch("hindsight_api.main.DefaultExtensionContext"), \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run"): # Don't actually start uvicorn
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_engine.return_value = MagicMock()
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
# Mock sys.argv to simulate CLI invocation
|
|
with patch.object(sys, 'argv', ['hindsight-api']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
# Verify TENANT extension was loaded
|
|
assert "TENANT" in loaded_extensions, \
|
|
"main.py did not call load_extension('TENANT', ...) - extensions not loaded!"
|
|
assert loaded_extensions["TENANT"] is not None, \
|
|
"load_extension('TENANT', ...) returned None despite env var being set"
|
|
assert isinstance(loaded_extensions["TENANT"], MockTenantExtension), \
|
|
f"Expected MockTenantExtension, got {type(loaded_extensions['TENANT'])}"
|
|
|
|
def test_main_loads_operation_validator_when_configured(self, monkeypatch):
|
|
"""
|
|
Verify that main.py loads operation validator from HINDSIGHT_API_OPERATION_VALIDATOR_EXTENSION.
|
|
"""
|
|
monkeypatch.setenv(
|
|
"HINDSIGHT_API_OPERATION_VALIDATOR_EXTENSION",
|
|
"tests.test_main_module:MockOperationValidator",
|
|
)
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
|
|
loaded_extensions = {}
|
|
|
|
from hindsight_api.extensions.loader import load_extension as real_load_extension
|
|
|
|
def tracking_load_extension(name, base_class):
|
|
result = real_load_extension(name, base_class)
|
|
loaded_extensions[name] = result
|
|
return result
|
|
|
|
with patch("hindsight_api.main.MemoryEngine") as mock_engine, \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.load_extension", side_effect=tracking_load_extension), \
|
|
patch("hindsight_api.main.DefaultExtensionContext"), \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run"):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_engine.return_value = MagicMock()
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
assert "OPERATION_VALIDATOR" in loaded_extensions, \
|
|
"main.py did not call load_extension('OPERATION_VALIDATOR', ...)"
|
|
assert loaded_extensions["OPERATION_VALIDATOR"] is not None
|
|
assert isinstance(loaded_extensions["OPERATION_VALIDATOR"], MockOperationValidator)
|
|
|
|
def test_main_passes_extensions_to_memory_engine(self, monkeypatch):
|
|
"""
|
|
Verify that main.py passes loaded extensions to MemoryEngine constructor.
|
|
|
|
This is the critical test - even if extensions are loaded, they must be
|
|
passed to MemoryEngine for authentication to work.
|
|
"""
|
|
monkeypatch.setenv(
|
|
"HINDSIGHT_API_TENANT_EXTENSION",
|
|
"tests.test_main_module:MockTenantExtension",
|
|
)
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
|
|
memory_engine_calls = []
|
|
|
|
def capture_memory_engine(*args, **kwargs):
|
|
memory_engine_calls.append({"args": args, "kwargs": kwargs})
|
|
return MagicMock()
|
|
|
|
with patch("hindsight_api.main.MemoryEngine", side_effect=capture_memory_engine), \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.DefaultExtensionContext"), \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run"):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
# Verify MemoryEngine was called
|
|
assert len(memory_engine_calls) == 1, "MemoryEngine should be called exactly once"
|
|
|
|
call_kwargs = memory_engine_calls[0]["kwargs"]
|
|
|
|
# THE CRITICAL ASSERTION: tenant_extension must be passed and not None
|
|
assert "tenant_extension" in call_kwargs, \
|
|
"MemoryEngine was not called with tenant_extension parameter!"
|
|
assert call_kwargs["tenant_extension"] is not None, \
|
|
"tenant_extension was None - main.py did not pass loaded extension to MemoryEngine!"
|
|
|
|
def test_main_sets_extension_context_on_tenant_extension(self, monkeypatch):
|
|
"""
|
|
Verify that main.py sets the extension context on tenant extension.
|
|
|
|
This is required for tenant extensions that need to provision schemas.
|
|
"""
|
|
monkeypatch.setenv(
|
|
"HINDSIGHT_API_TENANT_EXTENSION",
|
|
"tests.test_main_module:MockTenantExtension",
|
|
)
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
|
|
captured_tenant_ext = [None]
|
|
|
|
def capture_memory_engine(*args, **kwargs):
|
|
captured_tenant_ext[0] = kwargs.get("tenant_extension")
|
|
return MagicMock()
|
|
|
|
context_created = []
|
|
|
|
def capture_context(*args, **kwargs):
|
|
ctx = MagicMock()
|
|
context_created.append(ctx)
|
|
return ctx
|
|
|
|
with patch("hindsight_api.main.MemoryEngine", side_effect=capture_memory_engine), \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.DefaultExtensionContext", side_effect=capture_context), \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run"):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
# Verify context was created and set
|
|
assert len(context_created) == 1, "DefaultExtensionContext should be created"
|
|
assert captured_tenant_ext[0] is not None, "Tenant extension should be captured"
|
|
assert captured_tenant_ext[0]._context_set, \
|
|
"set_context was not called on tenant extension"
|
|
|
|
def test_main_works_without_extensions(self, monkeypatch):
|
|
"""
|
|
Verify that main.py works correctly when no extensions are configured.
|
|
"""
|
|
# Ensure no extension env vars are set
|
|
monkeypatch.delenv("HINDSIGHT_API_TENANT_EXTENSION", raising=False)
|
|
monkeypatch.delenv("HINDSIGHT_API_OPERATION_VALIDATOR_EXTENSION", raising=False)
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
|
|
memory_engine_calls = []
|
|
|
|
def capture_memory_engine(*args, **kwargs):
|
|
memory_engine_calls.append({"args": args, "kwargs": kwargs})
|
|
return MagicMock()
|
|
|
|
with patch("hindsight_api.main.MemoryEngine", side_effect=capture_memory_engine), \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run"):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
# Should work without extensions
|
|
assert len(memory_engine_calls) == 1
|
|
call_kwargs = memory_engine_calls[0]["kwargs"]
|
|
|
|
# Extensions should be None when not configured
|
|
assert call_kwargs.get("tenant_extension") is None
|
|
assert call_kwargs.get("operation_validator") is None
|
|
|
|
def test_main_uses_app_object_for_single_worker(self, monkeypatch):
|
|
"""
|
|
Verify that main.py passes the app object (not import string) when workers=1.
|
|
|
|
This is important because it means single-worker mode uses the app created
|
|
in main.py (with extensions loaded), not server.py.
|
|
"""
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "1")
|
|
monkeypatch.delenv("HINDSIGHT_API_TENANT_EXTENSION", raising=False)
|
|
|
|
uvicorn_calls = []
|
|
|
|
def capture_uvicorn_run(**kwargs):
|
|
uvicorn_calls.append(kwargs)
|
|
|
|
mock_app = MagicMock()
|
|
|
|
with patch("hindsight_api.main.MemoryEngine") as mock_engine, \
|
|
patch("hindsight_api.main.create_app", return_value=mock_app), \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run", side_effect=capture_uvicorn_run):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_engine.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api', '--workers', '1']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
assert len(uvicorn_calls) == 1
|
|
# With workers=1, should pass app object, not import string
|
|
assert uvicorn_calls[0]["app"] is mock_app, \
|
|
"main.py should pass app object (not import string) when workers=1"
|
|
|
|
def test_main_uses_import_string_for_multiple_workers(self, monkeypatch):
|
|
"""
|
|
Verify that main.py uses import string when workers > 1.
|
|
|
|
This is important because multi-worker mode requires server.py to be imported
|
|
by each worker process.
|
|
"""
|
|
monkeypatch.setenv("HINDSIGHT_API_WORKERS", "2")
|
|
monkeypatch.delenv("HINDSIGHT_API_TENANT_EXTENSION", raising=False)
|
|
|
|
uvicorn_calls = []
|
|
|
|
def capture_uvicorn_run(**kwargs):
|
|
uvicorn_calls.append(kwargs)
|
|
|
|
with patch("hindsight_api.main.MemoryEngine") as mock_engine, \
|
|
patch("hindsight_api.main.create_app") as mock_create_app, \
|
|
patch("hindsight_api.main._get_raw_config") as mock_get_config, \
|
|
patch("hindsight_api.main.print_banner"), \
|
|
patch("uvicorn.run", side_effect=capture_uvicorn_run):
|
|
|
|
mock_config = MagicMock()
|
|
mock_config.host = "0.0.0.0"
|
|
mock_config.port = 8888
|
|
mock_config.log_level = "info"
|
|
mock_config.mcp_enabled = False
|
|
mock_config.run_migrations_on_startup = False
|
|
mock_config.database_url = "postgresql://test:test@localhost/test"
|
|
mock_get_config.return_value = mock_config
|
|
mock_engine.return_value = MagicMock()
|
|
mock_create_app.return_value = MagicMock()
|
|
|
|
with patch.object(sys, 'argv', ['hindsight-api', '--workers', '2']):
|
|
from hindsight_api.main import main
|
|
main()
|
|
|
|
assert len(uvicorn_calls) == 1
|
|
# With workers > 1, should use import string
|
|
assert uvicorn_calls[0]["app"] == "hindsight_api.server:app", \
|
|
"main.py should use import string when workers > 1"
|
|
assert uvicorn_calls[0]["workers"] == 2
|
|
|
|
|
|
# Mock extensions for testing
|
|
from hindsight_api.extensions import (
|
|
OperationValidatorExtension,
|
|
RecallContext,
|
|
ReflectContext,
|
|
RequestContext,
|
|
RetainContext,
|
|
TenantContext,
|
|
TenantExtension,
|
|
ValidationResult,
|
|
)
|
|
|
|
|
|
class MockTenantExtension(TenantExtension):
|
|
"""Mock tenant extension for testing main.py extension loading."""
|
|
|
|
def __init__(self, config: dict):
|
|
super().__init__(config)
|
|
self._context_set = False
|
|
|
|
async def authenticate(self, request_context: RequestContext) -> TenantContext:
|
|
return TenantContext(schema_name="public")
|
|
|
|
async def list_tenants(self) -> list:
|
|
from hindsight_api.extensions.tenant import Tenant
|
|
|
|
return [Tenant(schema="public")]
|
|
|
|
def set_context(self, context) -> None:
|
|
self._context_set = True
|
|
|
|
|
|
class MockOperationValidator(OperationValidatorExtension):
|
|
"""Mock operation validator for testing main.py extension loading."""
|
|
|
|
def __init__(self, config: dict):
|
|
super().__init__(config)
|
|
|
|
async def validate_retain(self, ctx: RetainContext) -> ValidationResult:
|
|
return ValidationResult.accept()
|
|
|
|
async def validate_recall(self, ctx: RecallContext) -> ValidationResult:
|
|
return ValidationResult.accept()
|
|
|
|
async def validate_reflect(self, ctx: ReflectContext) -> ValidationResult:
|
|
return ValidationResult.accept()
|