Skip to content

feat(knowledge): config-driven per-workload pool effort + settings UI - #8345

Open
atomsbaza wants to merge 3 commits into
kirodotdev:mainfrom
atomsbaza:feat/knowledge-pool-effort-config
Open

feat(knowledge): config-driven per-workload pool effort + settings UI#8345
atomsbaza wants to merge 3 commits into
kirodotdev:mainfrom
atomsbaza:feat/knowledge-pool-effort-config

Conversation

@atomsbaza

@atomsbaza atomsbaza commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

Upstream PR #3944 isolated the Knowledge extraction and URL-fetch pools but hardcoded the policy: extraction always ran at DEFAULT_EXTRACTION_EFFORT = "high", overriding agent.role_efforts.background, and the review pipeline flagged the framing and mechanics (first-principles BLOCK, design CONCERNS):

  1. Hardcoded high ignores every operator effort knob (a test pinned that an explicit background: low was ignored).
  2. On the default auto model, kiro-cli rejects effort — the pool pushed /effort anyway, logging a warning on every worker spawn.
  3. _select_effort_level in llm_pool.py duplicated the effort ladder in providers/acp.py.
  4. use_config_pool_size patched the symptom of a workload-specific key inside the generic LLMPool; auto_research_llm_pool was still silently resizable by knowledge.extraction_pool_size.
  5. The knowledge_llm_pool alias + sync_source fallback had zero production consumers.
  6. docs/system-specs/modules/knowledge.md still documented the pre-feat(knowledge): isolate workload effort pools #3944 shared pool.

Why it matters

Operators pay real money for reasoning effort. Before this PR the operator had
no way to turn extraction effort down from the hardcoded high — every
document ingestion billed at high effort even when the material is routine,
and the setting was invisible in the UI. Meanwhile every worker spawn on a
non-reasoning model (the default auto) logged a failed effort push. After:
effort is a first-class settings control per workload, the default behavior is
byte-identical for operators who never touch it, and unsupported models stop
generating warning spam.

What changed (motivation → approach → change)

Config-driven effort (fixes 1)

  • New keys knowledge.extraction_effort and knowledge.fetch_effort (enum ["", low, medium, high, xhigh, max], default "").
  • Resolution per pool: explicit key → agent.role_efforts.background → fallback (high for extraction, provider default for fetch). Setting low now actually yields low; operators who never touch the keys keep today's behavior.
  • Model selection is unchanged.

Model-support gating (fixes 2)

  • AcpWorker._apply_effort checks model_supports_effort before pushing on the kiro backend; unsupported models (e.g. auto) skip with a single INFO line instead of a rejected /effort per spawn. The claude config-option path keeps its existing gating.

Ladder dedupe (fixes 3)

  • select_effort_level(requested, supported) now lives in kiro_crew.effort; llm_pool imports it. providers/acp.py keeps its trial-descend loop (that transport has no advertised list to read) — two transports, one ordering.

Pool-size decoupling + alias removal (fixes 4, 5)

  • use_config_pool_size replaced by an explicit config_pool_size_key; only the extraction pool passes it. auto_research_llm_pool is no longer silently resized by a Knowledge-section key.
  • knowledge_llm_pool alias and the sync_source fallback deleted; missing knowledge_fetch_pool now fails loudly.

Settings UI

  • New Knowledge section in Settings → Chat with four controls: Extraction model, Extraction Effort, URL Fetch Effort, Extraction pool size (restart-required hint). Built with the panel's SettingsSelect/SettingsInput primitives + configKey, so the generated settings registry picks them up and command-palette <SettingRef> chips deep-link.
  • The extraction-model and pool-size rows were removed from the Knowledge tab's settings (they now live in Settings → Chat); remaining tab controls are unchanged.
  • i18n: new strings mirrored across all 12 locales; npm run i18n:check passes, en-XA regenerated.

Docs (fixes 6)

  • docs/system-specs/modules/knowledge.md updated for the two-pool policy, the new keys, the resolution chain, and the model-support gate.

Tests

  • 823 passed / 1 skipped across test_llm_pool.py, test_knowledge_effort_pools.py, test_knowledge_sync_local_file.py, test_knowledge_handlers_coverage.py, test_config_loader.py — including the deliberate flip of the old pin: extraction_effort="low" now wins over the high fallback.
  • Frontend: i18n suite + KnowledgeSettingsTab + SettingsChatPanelCoverage green (749 tests in the touched areas); full npm test suite green after the fixes below.
  • flake8, targeted mypy, isort, black formatting gate, npm run build all pass.

Manual verification

  • Capture harness (website/scripts/capture-knowledge-effort.mjs) drives the real built SPA with fixture-backed APIs:
    • Settings → Chat, Knowledge section default state:
      default
    • With effort values picked (extraction low, fetch medium, pool size 5):
      picked
    • Knowledge tab with the moved rows removed:
      tab

Out of scope

  • CCWorker (claude_code harness) still takes no effort — pre-existing gap, harness-parity-neutral.
  • Live effort changes without restart (pools re-read config on respawn; same restart contract as extraction_pool_size).
  • Per-source effort overrides.

Related Issues

No tracked issue; addresses the review findings on #3944.

Add knowledge.extraction_effort / knowledge.fetch_effort (enum ['', low..max],
default '' = inherit). Resolution per pool: explicit key ->
agent.role_efforts.background -> fallback (extraction high, fetch provider
default), so an explicit low now wins where the old constant overrode it.

Gate effort application in AcpWorker on model_supports_effort for the kiro
backend: unsupported models (e.g. auto) skip the push with one INFO line
instead of a rejected /effort per worker spawn.

Replace the use_config_pool_size flag with an explicit config_pool_size_key,
so knowledge.extraction_pool_size no longer silently resizes the
auto_research pool. Drop the knowledge_llm_pool alias and the sync_source
fallback (no production consumers). Move select_effort_level to
kiro_crew.effort so the ladder lives in one place.

Update docs/system-specs/modules/knowledge.md for the two-pool policy.
Move the extraction-model and extraction-pool-size controls out of the
Knowledge tab's settings into a Knowledge section on Settings > Chat, and
add Extraction Effort / URL Fetch Effort selectors alongside them. The
panel's own SettingsSelect primitives feed the generated settings registry,
so the new controls deep-link from the command palette.

Capture harness + screenshots (docs/assets/pr-knowledge-effort/) cover the
default state, picked values, and the slimmed-down Knowledge tab.
@atomsbaza
atomsbaza requested a review from a team September 4, 2026 01:01
@atomsbaza
atomsbaza requested a review from a team as a code owner September 4, 2026 01:01
@atomsbaza
atomsbaza requested a review from bolichen97 September 4, 2026 01:01
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention labels Sep 4, 2026
The template requires capture media under the ephemeral
temp-screenshots/<feature>/ dir (never docs/) with SHA-pinned embed URLs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant