close
Skip to content

feat(knowledge): move settings to Knowledge page Settings tab - #3335

Merged
iamwhatever merged 1 commit into
mainfrom
feat/knowledge-settings-tab
Aug 14, 2026
Merged

feat(knowledge): move settings to Knowledge page Settings tab#3335
iamwhatever merged 1 commit into
mainfrom
feat/knowledge-settings-tab

Conversation

@iamwhatever

@iamwhatever iamwhatever commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Problem

Knowledge ingestion settings (per-source chunk limit, max sources, embed rate limit,
extraction model, pool size, auto-ingest toggles) lived in Settings → Chat, buried
among unrelated options. Users couldn't find them without knowing the exact path.

Why it matters

Knowledge cost control and model choice are per-feature decisions — they belong on
the Knowledge page where the user is already managing sources, not in a general
settings panel.

Fix (symptoms → root cause → change)

Symptom: settings hard to find. Root cause: wrong placement.
Change: move settings into a new "Settings" tab on the Knowledge page itself.

  • New SettingsTab.tsx component with 8 config fields (3 toggles + 5 numeric/dropdown)
  • Knowledge index.tsx gains 4th tab (List | Graph | Sources | Settings)
  • Remove entire Knowledge Library section from Settings → Chat panel (−89 lines)
  • Auto-ingest toggles default to false (opt-in, not opt-out)
  • Number() + Number.isInteger() for robust numeric validation
  • Config load error state with retry button
  • onError reverts all local inputs to server values

Fields exposed

Field Control Default
Auto-add documents toggle off
Auto-register project documents toggle off
Auto-add saved artifacts toggle off
Per-source chunk limit number input 150
Max sources number input 50
Embedding rate limit number input + /min 120
Extraction model dropdown (useAvailableModels) auto
Extraction pool size number input + restart badge 3

Tests

  • KnowledgeSettingsTab.test.tsx — 6 tests covering render, blur-commit validation,
    out-of-range revert, NaN revert, model selection, and error banner.

Manual verification

Pod screenshot captured via Playwright against an isolated pod instance.

Screenshots

Knowledge Settings tab

no issue closed: this is the frontend follow-up from PR #2468 which had no issue filed.

Follow-up to #2468.

@iamwhatever
iamwhatever requested a review from a team August 13, 2026 15:57
@iamwhatever
iamwhatever requested a review from a team as a code owner August 13, 2026 15:57
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of ce9b8fdf163a0cb555a176c28f56679d371e775c and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/integration/setup.ts:37 -- "if (isPostMswDial) return" suppresses genuine leaked dashboard-origin connection failures across all tests, an undocumented global harness change -> Fix: remove this hunk.
[GPT-REVIEWED] ce9b8fd

False positive or not applicable? A repository writer can comment:
/ai-review override gpt ce9b8fdf163a0cb555a176c28f56679d371e775c: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Advisory design-level review of ce9b8fdf163a0cb555a176c28f56679d371e775c — updated in place on each push; does not block merge.

Design-Verdict: CONCERNS

The move fixes browsing but silently removes these 8 settings from command-palette search — partially defeating the stated "users couldn't find them" goal.

Watch

  • Palette discoverability regression, same root cause as the parallel UI kit. The registry generator scans only src/pages/settings/*.tsx (gen-settings-registry.mjs: settingsDir = .../src/pages/settings); deleting the ChatPanel section removes the four registry entries (see the settingsRegistry.gen.ts hunk) and the new knowledge/SettingsTab.tsx — with its own hand-rolled SettingRow/Toggle/NumberInput instead of the shared SettingsToggle/SettingsInput — sits outside the extractor, SettingRef highlighting, and /settings?highlight= deep links. Consequence: a user typing "auto-add documents" or "embedding rate" in the palette now gets nothing, and any doc/tooltip that SettingRef-links these settings dead-ends. Either extend the extractor/registry to the new tab or add manual palette entries pointing at /knowledge?tab=settings.
  • Scope creep: the website/integration/setup.ts ECONNREFUSED-suppression hunk is test-infra unrelated to this PR's purpose and unmentioned in the description — move it to its own PR (it changes what failures the whole integration suite can surface).

Suggestions

  • Delete the 10 orphaned pages.settings.chatPanel.* knowledge keys from the locale files instead of raising the deadKeys BASELINE 21→31 — the ratchet exists precisely to catch copy deleted without its key.
  • Don't commit temp-screenshots/settings-tab.png to the repo; attach the screenshot to the PR instead of permanently shipping a 126 KB binary from a directory literally named "temp".

[DESIGN-REVIEWED] ce9b8fd

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

Advisory UX-level review of ce9b8fdf163a0cb555a176c28f56679d371e775c — updated in place on each push; does not block merge.

UX-Verdict: CONCERNS

A move made in the name of discoverability quietly deletes both existing discovery paths: ⌘K settings search and the old Settings → Chat location.

Watch

  • ⌘K can no longer find these settings. The diff removes all four knowledge entries from settingsRegistry.gen.ts, and the generator only scans src/pages/settings, so the new tab is never indexed. An existing user searches "auto-add documents" in the palette → zero results, checks Settings → Chat → gone, with no pointer to the new home. High frequency for habituated users, dead-end every time. Smallest fix: extend the registry generator to the Knowledge tab, or leave a one-line "Moved to Knowledge → Settings" link in Settings → Chat.
  • Consent-critical scope facts dropped from toggle copy. Old descriptions promised "never source code, agent instructions or generated files" and "appear in one 'Auto-added' source you can remove in a click"; new auto_project_desc/auto_add_desc omit both. A user deciding whether to let the agent auto-ingest their files loses the exact facts that make the decision safe. Restore the scoping and reversibility sentences.
  • Hand-rolled Toggle has no accessible name. Three role="switch" buttons render with only aria-checked; a screen-reader user hears "switch, off" ×3 and cannot tell them apart — the existing SettingsToggle/Toggle primitives in components/settings.tsx already wire label and whole-row click. Reuse them.
  • Out-of-range input snaps back silently. commitNumber reverts e.g. 20000 → 150 on blur with no message and no visible max; the user may not notice their value never saved. State the range in the helper text or show the save-failed notice on revert.

Suggestions

  • model_auto option "auto (use chat model)" → capitalize "Auto" to match its own description and sibling labels.

[UX-REVIEWED] ce9b8fd

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed ce9b8fdf163a0cb555a176c28f56679d371e775c — this comment is updated in place on each push.

Review details

Now I have enough to falsify each candidate.

Candidate 3 — the model-literal gate at code-review.yml:163 scans only ':(glob)src/kiro_crew/**/*.py'. The claude-haiku-4.5 example lives in website/src/i18n/locales/*.json, which the gate never touches, so the CI-trip theory is false. The remaining "stale example" concern is low-confidence advisory copy/style — drops.

Candidate 4 — raising the dead-key BASELINE is dead-code/lint hygiene, a category this pipeline owns deterministically; excluded from findings regardless of accuracy.

Candidates 1 & 2 — verified real: SettingRow renders its label in a bare <div> with no htmlFor/id/aria-labelledby; Toggle (<button role="switch">) contains only a decorative <span> with no aria-label; NumberInput is a bare <input type="number"> with no aria-label. The sibling SimpleSelect in the same file got an explicit aria-label, but the toggles and number inputs did not, so they announce with no name. Same root cause → merge.

Neither blocking rule cleanly applies: accessible-interactive-elements regex targets <div|span onClick (these are <button>/<input>), and icon-buttons-need-labels scopes to buttons "that contain only icons (Lucide components)" — the toggle contains a styled span, not a Lucide icon, and inputs aren't buttons. So it's a genuine gap but not a violation of a blocking: true rule → advisory.

No blocking issues; one advisory accessibility gap in the new Knowledge settings controls.

FINDING — website/src/pages/knowledge/SettingsTab.tsx:1009 — the new Toggle <button role="switch"> (and the four NumberInput <input type="number"> at :989) carry no aria-label and SettingRow's label <div> isn't associated, so a screen reader announces them with no name — unlike the sibling SimpleSelect, which got aria-label, and unlike the old SettingsToggle this replaced → Fix: thread label into Toggle/NumberInput as aria-label, or give SettingRow an id/htmlFor association.

[OPUS-REVIEWED] ce9b8fd

Verdict parsed from the review's SHA-scoped output markers for commit ce9b8fdf163a0cb555a176c28f56679d371e775c.

False positive or not applicable? A repository writer can comment:
/ai-review override fable ce9b8fdf163a0cb555a176c28f56679d371e775c: <one-sentence reason>

@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from c772e52 to eba15ff Compare August 13, 2026 16:33
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from eba15ff to 7525c4e Compare August 13, 2026 16:49
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from 7525c4e to fd291ad Compare August 13, 2026 17:00
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from fd291ad to 7e26e38 Compare August 13, 2026 17:13
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from 7e26e38 to 3346ea2 Compare August 13, 2026 17:33
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from e0e0dab to 939467c Compare August 13, 2026 19:23
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from 939467c to 9d9e34b Compare August 13, 2026 23:06
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from 9d9e34b to 423cf63 Compare August 13, 2026 23:44
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 13, 2026
Add a dedicated Settings tab to the Knowledge page exposing 5 ingestion
config fields: per-source chunk limit, max sources, embedding rate limit,
extraction model (dropdown), and extraction pool size.

- New SettingsTab.tsx component with commit-on-blur number inputs and
  model dropdown using useAvailableModels()
- Knowledge index.tsx gains 4th tab (list | graph | sources | settings)
- Remove entire Knowledge Library section from Settings → Chat panel
  (auto-ingest toggles remain config-only for power users)
- i18n: 18 new keys across 14 locales + en-XA pseudolocale
- Remove obsolete ChatPanel.knowledgeAutoIngest test file
- Remove dead Knowledge Library test block from SettingsChatPanelCoverage
- Update deadKeys baseline (21 → 31) for removed ChatPanel keys

Closes the frontend follow-up from PR #2468.
@iamwhatever
iamwhatever force-pushed the feat/knowledge-settings-tab branch from 423cf63 to ce9b8fd Compare August 14, 2026 00:14
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 14, 2026
@iamwhatever
iamwhatever enabled auto-merge (squash) August 14, 2026 05:07
@iamwhatever
iamwhatever merged commit 456e199 into main Aug 14, 2026
89 of 91 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants