Skip to content

feat(customize): register Knowledge's Sources view and Add Source with find_ui - #23324

Open
cixuuz wants to merge 1 commit into
kirodotdev:mainfrom
cixuuz:feat/customize-control-deeplink
Open

cixuuz wants to merge 1 commit into
kirodotdev:mainfrom
cixuuz:feat/customize-control-deeplink

Conversation

@cixuuz

@cixuuz cixuuz commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: Asked how to add a source to the knowledge base, the agent can walk the user straight to the Add Source button in their own dashboard, not just to the Knowledge tab.

Asked where the knowledge library lives, the agent can only link to the tab (/capabilities?tab=knowledge, which find_ui returns). Sent to "Add Source", the user lands on the Knowledge list and has to find the Sources view and the button. Settings links ring the control; the Settings scheme stops at Settings (settings-deeplink.md: Customize, Developer and the App Store are not enumerated).

Why it matters

"Where do I add a folder to the knowledge base?" is a common question, and the answer today is a tab plus a description of where to click. An answer that lands on the control removes that step, and find_ui can only give one for a control it has registered.

Not a goal

  • New ?highlight= routes or any other agent-facing link format: find_ui's existing guide is the contract.
  • Customize controls beyond Knowledge's Sources view and Add Source.
  • Registering the Knowledge search box: it renders through SearchInput, which does not forward a find_ui marker yet.

What changed (motivation → approach → change)

The first revision added its own ?highlight=key: routes and an agent doc listing them. Review (First Principles item 5, the Design note, and the maintainer hold) pointed out that this was a second hand-kept list next to the find_ui index, which already registers Customize controls under a parent tab (connections.mcp-servers-tab → mcp.add-custom). This revision drops that list and registers the two Knowledge locations in find_ui instead. The agent already queries find_ui, and its guide walks the user's own dashboard tab to the control.

  • uiLocations/areas/capabilities.ts: knowledge.sources-tab (parent tab.capabilities.knowledge) and knowledge.add-source (parent knowledge.sources-tab), with newcomer search terms in en and zh-CN.
  • pages/knowledge/index.tsx: the Sources tab gets its own branch in the tab strip so its marker and label key are written at the render site; the other tabs render as before. The view content is wrapped in a GuideRevealScope for open:knowledge.sources-tab, the scope the generated plan names, so the guide can tell "Sources is open" from "Add Source hasn't loaded yet" (same pattern as Connections' MCP Servers tab).
  • pages/knowledge/SourcesList.tsx: Add Source carries the knowledge.add-source marker.
  • ui-index.generated.json and guidePlans.gen.ts are regenerated with npm run gen:ui.

find_ui("add source") now returns Customize › Knowledge › Sources › + Add Source with a guide plan; before, it stopped at the Knowledge tab.

Backwards compatibility

Compatible: two new index entries and two markers; no existing location, route or label changes.

Removes nothing: the tab strip still renders the same four tabs with the same labels and handlers.

Tests

  • uiLocations/schedule-artifacts-capabilities.locations.test.ts: goldens for both new locations (kind, label key, parent chain, route, entry, requirements) and that each is marked at exactly one render site. Removing the Add Source marker fails it.
  • KnowledgeEmbedded.test.tsx: the page reports open:knowledge.sources-tab closed on the list view, open after clicking Sources, and drops the report on unmount. With the GuideRevealScope removed, the test fails.
  • The uiLocations suite, guide/revealScopes.test.tsx and the Knowledge page tests pass: 17 files, 487 tests, 3 runs with --sequence.shuffle.
  • Backend: the 40 test files that read the UI index, guide catalog or find_ui pass (2205 tests).
  • Ran this branch's find_ui against the regenerated index: "add source", "add a folder to the knowledge base" and "knowledge sources" all resolve to the new locations with a guide.

Manual verification

N/A: the change is index data plus two DOM markers; the query results above come from running this branch's backend find_ui.

Screenshots / video

Why no screenshot: the Sources tab and Add Source render exactly as before; the diff adds only find_ui markers and index entries.

Related Issues

Part of #23304

The issue asks for ?highlight= links; this PR meets the same need through find_ui instead, so whether the issue is done is a maintainer's call.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@cixuuz
cixuuz requested review from a team and KiroCrewCaptain as code owners October 23, 2026 04:12
@cixuuz
cixuuz requested a review from dodgeblaster October 23, 2026 04:12
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: checking Automated validation is still running labels Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 73420d4e74f054dce976d59eaa94699bd457b465 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks: the two locations go into the existing find_ui registry, with the same parent-tab and reveal-scope pattern as connections.mcp-servers-tab, and the page renders the same as before.

Notes

  • The ## Backwards compatibility section has both a Compatible: line and a Removes nothing: line; keep only Removes nothing:, because the patch takes nothing away.

[DESIGN-REVIEWED] 73420d4

Description read

[DESCRIPTION-READ] 9c5e9bbb202903fe0d725f9d6bcec062ff27c7dd6d88ffa894fe3d4addb1e815

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5, fork) — ✅ PASS

UX-level review of 73420d4e74f054dce976d59eaa94699bd457b465 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

Nothing to check. The diff adds find_ui markers, search terms and a context-only GuideRevealScope. The Sources tab keeps the same icon, label key and handler, and Add Source keeps its markup, so nothing on screen changes.

[UX-REVIEWED] 73420d4

Description read

[DESCRIPTION-READ] 19fa9ee8ebbe6b17cac618c5ff5c44cb973a4e16bde567f5bbd1cdadbd7f4849

That sha256 names everything this verdict read: the PR description, plus the 3 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 73420d4e74f054dce976d59eaa94699bd457b465 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. The PR registers two locations with the find_ui index that already exists, using the reveal-scope pattern Connections uses for MCP Servers (ConnectionsPage.tsx:693), and changes no default.

What this change ships

Inventory (6 items) — 6 justified

Intent: when asked how to add a source to the knowledge base, the agent walks the user to the Add Source button itself, not just to the Knowledge tab. This is an ADDITION.

  1. find_ui("add source") and similar newcomer questions now resolve to Customize › Knowledge › Sources › + Add Source, with a guide plan — justified
  2. The Knowledge Sources view tab can now be found as its own location through find_ui — justified
  3. The guide can tell when the Sources view is open (reveal scope open:knowledge.sources-tab) — justified
  4. The Sources tab now renders through its own branch in the tab strip and looks the same — justified
  5. New English and Chinese (zh-CN) search terms for both locations — justified
  6. The packaged UI index, its label strings in 12 locales and the guide plans are regenerated — justified

[FIRST-PRINCIPLES-REVIEWED] 73420d4

Description read

[DESCRIPTION-READ] 765ea5286af9fbdf5e610766e5987ae22d720b99d1c0bbc2868a142e4602ffe9

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 73420d4e74f054dce976d59eaa94699bd457b465 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 73420d4

@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from 3c10907 to d3c1bda Compare October 23, 2026 04:27
@cixuuz

cixuuz commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author
  • A second highlight mechanism next to useSettingHighlight (span=b146a7705e4e)

Outcome: fixed in d3c1bda, the way the Clears-when line asks.
Customize now rings its controls through useSettingHighlight's key: path: CapabilitiesPage mounts useSettingHighlight, each control carries a data-setting-key pseudo-key (customize-knowledge-search, customize-knowledge-add-source), and those anchors join LATE_MOUNT_ANCHORS beside default-crewmate. useCustomizeHighlight, deepLinkFlash.ts and data-customize-id (with its isGuideMarkerAttr change) are gone; useSettingHighlight differs from main only by those two anchors in the late-mount set.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed 73420d4e74f054dce976d59eaa94699bd457b465 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 73420d4

@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from d3c1bda to d1d3f7a Compare October 23, 2026 16:26
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Oct 23, 2026
@bolichen97

Copy link
Copy Markdown
Contributor

Human design review is required before this PR can merge. Customize deep links make agent-facing routes and highlight targets part of the public product contract. A maintainer must approve that contract for this head. This comment records the design gate only; it is not a code defect.

@iamwhatever

Copy link
Copy Markdown
Contributor

Hold at d1d3f7a.

The code reads correct. Customize reuses useSettingHighlight, the two anchors wait in the late-mount set, and ?view= is checked against the known tabs and then removed. Every check is green or skipped. Current-head review notes are still unanswered:

  1. First Principles item 5 and the Design note: customize-deeplink.md is a second hand-kept list of Customize controls next to the find_ui index in website/src/uiLocations/areas/capabilities.ts, and the two will drift apart. find_ui("add source") still stops at the tab. Please register the anchors in the find_ui index, or explain why a separate doc is the better contract.
  2. First Principles item 6: customizeRoute() has no caller outside its test, and customizeAnchor() only wraps data-setting-key. Please reply with a disposition, or replace both with exported constants.

A maintainer also needs to accept these routes as an agent-facing contract, as noted in the earlier comment.

— crew-pr-reviewer (r123)

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Oct 24, 2026
@cixuuz cixuuz changed the title feat(customize): deep links that open a Customize tab and flash a control feat(customize): register Knowledge's Sources view and Add Source with find_ui Oct 24, 2026
@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from d1d3f7a to ca0d2f8 Compare October 24, 2026 06:58
@cixuuz

cixuuz commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Re the hold at d1d3f7a (crew-pr-reviewer r123): both items are addressed in ca0d2f8 by taking the first option.

  1. Second list next to find_ui (First Principles item 5, Design note). The anchors are now registered in the find_ui index: knowledge.sources-tab (under tab.capabilities.knowledge) and knowledge.add-source (under knowledge.sources-tab) in website/src/uiLocations/areas/capabilities.ts, following the connections.mcp-servers-tab → mcp.add-custom pattern. customize-deeplink.md and its README/index entries are gone. Running this branch's find_ui("add source") now returns Customize › Knowledge › Sources › + Add Source with a guide plan, instead of stopping at the tab.
  2. customizeRoute() / customizeAnchor() (First Principles item 6). Both are deleted along with customizeDeepLinks.ts, and so are the ?highlight=key: late-mount anchors and the ?view= reader that existed only to serve them.

Since the agent-facing contract is now find_ui's existing one, this PR no longer adds routes that need maintainer sign-off.

Tests: goldens for both locations in schedule-artifacts-capabilities.locations.test.ts (exact parent chain, label key, one render site; removing the Add Source marker fails them). The uiLocations suite passes 410/410 three times shuffled, and the 40 backend files that read the index pass (2205 tests).

The frozen Goal and the last Not-a-goal line still describe the first approach; they're noted as such in the description until a human edits them.

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Oct 24, 2026
@iamwhatever

Copy link
Copy Markdown
Contributor

HOLD (needs a maintainer decision) at ca0d2f8.

The code is sound. The two Knowledge locations are registered in the find_ui index under the Knowledge tab, following the existing connections.mcp-servers-tab → mcp.add-custom pattern, and the Sources tab keeps the same icon, label and handler. Both r123 items are answered: the separate deep-link doc and the uncalled helpers are gone. All AI lanes pass and every check on this head is green or skipped.

What still needs a human:

  1. The PR says Closes #23304, but #23304 asks for ?highlight= deep links that open a Customize tab and flash a control. This head ships find_ui guide entries instead. A maintainer should decide whether that proposal is superseded (then closing it is right) or still wanted (then use Part of #23304).
  2. The frozen Goal and the last Not-a-goal line still describe the dropped link approach, as the Design and First Principles notes say. A maintainer should edit them so the record matches what ships.
  3. @bolichen97's design-gate comment was written for the earlier route contract. A maintainer should confirm it no longer applies now that no new routes ship.

Minor: index.tsx line 577 has lost its indentation.

— crew-pr-reviewer (r124)

@cixuuz

cixuuz commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Re the hold at ca0d2f8 (crew-pr-reviewer r124): all four items are handled in f0fd54d, which is also rebased onto current main.

  1. Closes #23304. Changed to Part of #23304. The issue asks for ?highlight= links; this PR meets the same need through find_ui instead, so the PR no longer closes the issue on its own. A maintainer can close it if they consider the proposal superseded.
  2. Frozen Goal and Not-a-goal. Rewritten at the author's request to match what ships. The Goal is now walking the user to Add Source through find_ui. The Not-a-goal list now excludes new ?highlight= routes, controls beyond the Sources view and Add Source, and the search box (SearchInput doesn't forward a find_ui marker yet). The "frozen sections are stale" note is gone.
  3. @bolichen97's design gate. That gate was raised because the earlier revision made ?highlight= routes and highlight targets part of the agent-facing contract. None ship now: customize-deeplink.md, customizeDeepLinks.ts and the ?highlight=key: and ?view= code were removed in ca0d2f8. The only additions are two entries in the existing find_ui index, which is already the contract agents use. @bolichen97, could you confirm the gate no longer applies?
  4. Indentation at index.tsx:577. Fixed.

Also in this push: the Knowledge view content now sits in a GuideRevealScope for open:knowledge.sources-tab, the scope the generated plan already names, so the guide's live observation can report the Sources view open or closed (same pattern as Connections' MCP Servers tab). A rendered test in KnowledgeEmbedded.test.tsx covers it and fails without the owner.

Checks run locally on f0fd54d: gen:ui reports the index current, typecheck and ESLint pass, the uiLocations, reveal-scope and Knowledge tests pass (487 of 487, three shuffled runs), and the 40 backend files that read the index pass (2205 tests).

@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from ca0d2f8 to f0fd54d Compare October 24, 2026 22:56
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Oct 24, 2026
@cixuuz

cixuuz commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

Bundle Size Gate is red on f0fd54d, and it is pre-existing on main, not caused by this PR.

The failing line from the job:

FAIL assets/ModelEffortDropdown-CQlL9qDo.js: 505.0 KB exceeds its 500.0 KB budget by 5.0 KB (chunk 'ModelEffortDropdown')

I ran the same two steps CI runs (vite build --mode analyze, then node scripts/check-bundle-size.mjs) on this PR's base, main at 1a7190c, with no PR changes:

FAIL assets/ModelEffortDropdown-C8urXd7x.js: 505.0 KB exceeds its 500.0 KB budget by 5.0 KB (chunk 'ModelEffortDropdown')

The chunk is the same size with and without this diff, and this PR changes no file in that chunk's import graph beyond the Knowledge page markers. So the budget was crossed on main, and this PR is not the place to raise it or split the chunk. I didn't find an open issue for it.

@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 Oct 25, 2026
@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from f0fd54d to c21ffa5 Compare October 25, 2026 04:18
@cixuuz

cixuuz commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Correction to my earlier Bundle Size Gate comment. Two statements in it were wrong:

  • "I didn't find an open issue for it." The budget had already been fixed on main by fix(ci): re-measure the ModelEffortDropdown chunk's bundle-size ceiling #23867 (fix(ci): re-measure the ModelEffortDropdown chunk's bundle-size ceiling), which gives the chunk its own 531 KB ceiling. That PR landed after this branch's previous base (1a7190c), which still used the 500 KB default.
  • "this PR changes no file in that chunk's import graph beyond the Knowledge page markers." I didn't check which modules that chunk contains, so I shouldn't have said that.

The conclusion that the failure isn't caused by this PR still holds: the chunk measured 505.0 KB on the base alone.

The branch is now rebased onto current main (which includes #23867) at c21ffa5. The patch is unchanged (git range-diff shows =). Running CI's two steps locally on it (vite build --mode analyze, then node scripts/check-bundle-size.mjs) gives: 927 chunks within budget (default 500.0 KB, 25 allowlisted).

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 25, 2026
@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from c21ffa5 to 68fde47 Compare October 25, 2026 05:55
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Oct 25, 2026
@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from 68fde47 to 466a753 Compare October 25, 2026 17:34
@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 merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Oct 25, 2026
@cixuuz
cixuuz force-pushed the feat/customize-control-deeplink branch from 466a753 to 73420d4 Compare October 25, 2026 19:11
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Oct 25, 2026

This branch has not been deployed

No deployments
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) merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants