feat(config): make store_document_text overridable per bank - #2940
Merged
Conversation
HINDSIGHT_API_STORE_DOCUMENT_TEXT was static/server-level. Make it hierarchical so a data-minimizing bank (e.g. GDPR-sensitive) can keep only derived facts while other banks on the same deployment retain the raw source. - Add store_document_text to _CONFIGURABLE_FIELDS (settable per bank via the config API's generic updates dict, like audit_log_enabled). - Thread the per-bank resolved value into the retain storage path (chunk_storage.store_chunks_batch + fact_storage.upsert_document_metadata / handle_document_tracking / _upsert_document_row) from the orchestrator's resolved config; falls back to the server-level config when unset so non-retain callers (import) are unchanged. - Make the three consistency guards per-bank too so a store-off bank behaves coherently: append-mode rejection, recall include_chunks force-off, and the reflect 'expand' tool exclusion. - Docs: mark the flag hierarchical. Covered by a per-bank override test (one bank off, one default-on) + a configurable-fields guard; existing global-flag tests set the ConfigResolver global snapshot (env alone no longer suffices for a hierarchical field, mirroring enable_audit_default).
…tes & UI - BankTemplateConfig gains store_document_text and audit_log_enabled so bank templates can preset them; regenerated bank-template-schema.json. - Control-plane bank config: new 'Document Storage' tri-state section (Inherit / On / Off), mirroring the audit toggle; translations added across all 10 locales (non-en use English placeholders pending translation). Backend template round-trip + messages parity/used-keys + tsc all green.
…ivacy' section Combine the separate Audit Logging and Privacy config sections into a single Security & Privacy section with both tri-state toggles and one save (writes audit_log_enabled + store_document_text together). Drop the now-unused section-level message keys across all locales; add securityPrivacy* keys.
store_document_text became bank-configurable, so get_config().store_document_text now raises ConfigFieldAccessError (the guard forcing per-bank resolution). The storage functions' None-fallback hit that guard, breaking every direct/delta caller that didn't pass the value (test_chunk_storage_upsert, test_delta_retain). Fall back to _get_raw_config() instead — the unguarded global layer the ConfigResolver and the /config defaults response already use. The retain path still passes the per-bank resolved value; only non-retain callers hit the fallback.
…ig fields Adding store_document_text/audit_log_enabled to BankTemplateConfig changed the OpenAPI schema; regenerate the spec, Go/Python/TS client models, and docs-skill copies, and apply lint formatting (verify-generated-files).
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.
Summary
Makes
HINDSIGHT_API_STORE_DOCUMENT_TEXToverridable per bank. It was astatic, server-level flag; now a data-minimizing bank (e.g. a GDPR-sensitive
one) can keep only derived facts —
documents.original_textNULL,chunks.chunk_textempty — while other banks on the same deployment retain theraw source for expansion / re-extraction.
What changed
store_document_textadded to_CONFIGURABLE_FIELDS,so it's settable per bank through the config API's generic
updatesdict(same mechanism as
audit_log_enabled— no typed-model change needed).orchestrator's resolved
configintochunk_storage.store_chunks_batchandfact_storage.{upsert_document_metadata, handle_document_tracking, _upsert_document_row}. The new param falls back to the server-level configwhen unset, so non-retain callers (e.g. import) are unchanged.
(previously these read the global flag):
retain_batch_async) — resolves per bank only whenan append is actually requested;
include_chunksforce-off (no raw chunk text to attach);expandtool exclusion (it only reads back raw source text).configuration.md.Notes for review
/config-style) intentionally still reports theglobal default — the per-bank value lives on the bank config.
ConfigResolverglobalsnapshot: the resolver snapshots the global layer at construction, so flipping
the env var alone is not enough for a hierarchical field (mirrors the existing
enable_audit_defaulthelper).Test plan
test_store_document_text.py::test_store_document_text_per_bank_override—one bank set
store_document_text=false, one left default: the first dropsraw doc/chunk text (facts still stored), the second keeps it. Real Postgres.
test_store_document_text_is_bank_configurable— the field is in theconfigurable set.
expand-tool tests still pass (fixture updated for the hierarchical layer).
test_retain.pymock-LLM suite green (orchestrator threading unaffected);ruff+tyclean.