fix(knowledge): make the three auto-ingest paths opt-in - #2448
Conversation
Auto-add documents, auto-register project docs and auto-ingest artifacts all defaulted on, so a fresh install started writing to the Knowledge Library and spending LLM extraction calls without being asked.
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — 🟡 CONCERNSAdvisory UX-level review of UX-Verdict: CONCERNS Flipping the defaults is right, but it moves two silent-failure moments onto the very flow the flip creates: the first opt-in. Watch
[UX-REVIEWED] 70f68ec |
GPT 5.6 Review — 🔴 changes requested (blocking)GPT 5.6 found at least one blocking issue that must be resolved before merging This comment is updated in place on each push. BLOCKING -- src/kiro_crew/config/loader.py:5070 -- Missing-key upgrades leave later artifact opt-in stale |
Design Review (Fable 5) — 🟡 CONCERNSAdvisory design-level review of Design-Verdict: CONCERNS Right fix at the right layer (all six default sites flipped together), but it silently changes behavior on existing installs and routes new opt-ins into two known-broken paths. Watch
Suggestions
[DESIGN-REVIEWED] 70f68ec |
|
Dispositions for GPT 5.6 — BLOCKING, I verified it against
My own local review reasoned about the fresh-install case and missed the upgrade case where the row pre-exists. That was the gap. I do not think the suggested fix (keep the
Design Review (Fable 5) — 🟡 CONCERNS, "existing installs silently lose auto-ingest; loader has write-back machinery at UX Review (Fable 5) — 🟡 CONCERNS, "Auto-Add Saved Artifacts now silently does nothing when switched on" — ACCEPTED. This is the startup-read issue I disclosed in the PR body under "Known consequences" (§1). Under opt-in it stops being an edge case and becomes what every adopter hits once. It needs either a live config re-read on the artifacts path (matching its two neighbours in the same card, which re-read every watcher sweep) or an explicit restart hint on the toggle. Opus 4.8 — ✅ no findings. Nothing to disposition. Follow-up covering all three accepted items is next; I will link it here. |
Auto-add documents, auto-register project docs and auto-ingest artifacts all defaulted on, so a fresh install started writing to the Knowledge Library and spending LLM extraction calls without being asked. (cherry picked from commit bdd55fc)
|
Follow-up opened: #2452 — takes the reconcile-on-start route rather than restoring the |
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution.
…ble extraction model (#2468) The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (#2175, #2336, #2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution. Co-authored-by: Joe Guo <zejiangg@amazon.com>
…ble extraction model (kirodotdev#2468) The per-source chunk budgets (auto_ingest_chunk_budget, folder_ingest_chunk_budget) bound cost per folder per sweep, but with many directories registered the total work per sweep = N × per-source budget -- causing LLM pool saturation, embedding backlogs, and gateway stalls (kirodotdev#2175, kirodotdev#2336, kirodotdev#2448). Add five new KnowledgeConfig fields: - sweep_chunk_budget (default 500): hard cap on total chunks across ALL sources in one watcher sweep. Once reached, remaining sources defer to next sweep. - max_sources (default 50): cap on registered source count. Auto-discovery paths stop registering once the cap is reached. - embed_rate_limit (default 120/min): token-bucket throttle on embedding generation, preventing CPU/memory saturation from parallel embed batches. - extraction_model (default empty = agent.model): extraction LLM is no longer hardcoded to claude-haiku-4.5; it uses the user's default model, overridable. - extraction_pool_size (default 3): configurable concurrent extraction workers. All five are live-reloaded (no restart needed except pool size) and exposed in the Settings PATCH schema. 0 = unbounded for all numeric fields. Implementation: - watcher._scan() tracks chunks_used across all folder sources, breaking when the global budget is exhausted - folder_watcher.scan_source() reports chunks_ingested in stats - store.create_auto_source_unless_dismissed() enforces max_sources atomically - EmbedRateLimiter token bucket in ingestion.py, called from _embed_item() - _install_knowledge_agent() reads extraction_model from config - LLMPool.start() reads extraction_pool_size from config - handlers/core.py allows PATCH for all 5 new keys Tests: 24 new tests covering config defaults, rate limiter, source cap, sweep budget, pool size config, and extraction model resolution. Co-authored-by: Joe Guo <zejiangg@amazon.com>
Problem
A fresh install starts filling the Knowledge Library on its own. Three separate
auto-ingest paths default to on:
knowledge.auto_register_project_docsknowledge.auto_add_documentsknowledge.auto_ingest_artifactsEach ingested chunk costs one LLM extraction call on a pool of billed sessions,
so the user's first hour with a project spends money on a Library they never
asked for — and on a large repository the same automatic machinery is what put
the gateway into a stall/respawn loop (fixed separately in #2175 and #2336).
The toggles already existed. Their defaults were the problem.
Why it matters
The cost and the writes are both invisible until after they happen. A user who
opens a monorepo gets a source registered, chunks extracted, and a scheduled
dedup pass running against their Library before they have opened the Knowledge
page once. "Auto-ingest is on unless you find and flip three switches" is the
wrong side of the default for anything that spends money and mutates a store.
Fix (symptoms → root cause → change)
Symptom: the Library grows and extraction calls are spent with no user
action.
Root cause: the three gate flags default to
True, so the opt-out is theonly control and it is opt-out by construction.
Change: flip all three to
False. The features, their gates, their UItoggles and their budgets are otherwise untouched — this changes only which side
of the switch you start on.
Each flag has more than one default source, and flipping one in isolation
would make
KnowledgeConfig()andKiroCrewConfig.load()disagree, so all sixwere changed:
KnowledgeConfig.auto_*).get(..., True)fallbacks forauto_ingest_artifactsand
auto_register_project_docs_read_auto_add_documents's no-key-present fallback (the legacy-spellingreader)
auto_ingest_doc_links(the legacy spelling) still wins when a config sets itexplicitly, so an existing config that opted in keeps its value — only
configs that never mentioned the key change behavior.
The frontend carries its own copy of each default (
?? trueinChatPanel.tsx) and was flipped to match. Left alone, the switch would haverendered ON while the gateway ingested nothing, with no key in
config.jsonforthe user to inspect and settle which one was telling the truth.
Tests
website/src/test/ChatPanel.knowledgeAutoIngest.test.tsx(new, 6 cases) —pins the UI side of the default pair: with no
knowledgesection in theconfig at all, all three switches render
aria-checked=false, and the firstclick writes
true. Both assertions wait for the row to leave itsdisabledloading state first, so they cannot pass on a placeholder. Verified
non-vacuous: restoring one
?? trueturns it red.test/test_config_loader.py—test_auto_add_documents_defaults_off,test_project_docs_defaults_off,test_artifact_ingest_defaults_off, plustest_an_empty_knowledge_section_leaves_every_auto_path_off, which pins allthree together because they arrive through three different readers.
test/test_knowledge_artifact_ingest.py—TestKnowledgeConfigDefaults.test_auto_ingest_defaults_offasserts thedataclass default and the loader agree.
test_legacy_spelling_is_honoured,test_canonical_wins_over_legacy,test_round_trip_settles_on_the_canonical_key)are unchanged and still pass — they set the key explicitly, which is exactly
the upgrade path that must not regress.
Manual verification
Full gates from the worktree venv (CI-parity, no faiss, mypy 1.14.1 matching the
pyproject.tomlpin):pytest(full backend)isort --check-only/flake8mypy src/kiro_crew/hooks.pytsc -bvitest run(full frontend)vitest run src/i18n/scripts/docs-lint.shThe 17 backend failures and the 3 mypy errors are pre-existing and
host-specific, not from this diff:
origin/maincheckout usingthe same interpreter (
PYTHONPATHpointed at the baseline tree) and all 17failed there identically. They are macOS-only: Seatbelt sandbox spawn,
nested-pytest harness, a macOS temp-path redaction artifact, and a
__CF_USER_TEXT_ENCODINGenv leak.os.listxattr/getxattr/setxattrinhooks.py,which do not exist on macOS.
hooks.pyis not in this diff.No i18n catalog changes were needed: the three toggle descriptions never
claimed "on by default", so no locale string changed and the 11-locale parity
gate is untouched. Only backend metadata text and docs prose moved.
Screenshots
Captured against an isolated instance serving this branch's built bundle
(
dev-backend.shon its own port with its own data home, torn down afterwards).The capture script asserted
aria-checked=falseon all three switches beforeshooting, so a stale bundle fails loudly instead of being quietly photographed.
Full Settings → Chat page, in context
Known consequences, disclosed rather than hidden
Two pre-existing rough edges that this change moves onto the default path.
Both are out of scope here (each needs work inside the artifact ingest pipeline,
not the config layer) and I will file a follow-up issue:
_start_artifact_ingest_asyncreadsauto_ingest_artifactsonce at startup,whereas its two neighbours in the same card re-read config every watcher
sweep and apply immediately. Previously the flag shipped on, so nobody met
this; now it is the one switch in the card that appears to do nothing until
restart, with no hint in the UI.
backfill_artifactsiterates every eligible artifact with no chunk budget. Under the old
default it ran at first boot against an empty store; now it runs the first
time a user opts in, when the store may hold hundreds of artifacts — a burst
of extraction calls with no pacing. (
auto_ingest_chunk_budgetpaces foldersources, not this path.)
Not included
No
CHANGELOG.mdentry. This is a behavior change for existing installs — auser on 0.2.0 who never set these keys loses auto-ingest on upgrade and deserves
a release note — but
0.2.0is already tagged and shipped, and the repo keeps noUnreleasedsection, so I did not edit a released section unilaterally. Saywhere you want it (a new
0.2.1section now, or folded into the next release)and I will add it.