Repository navigation
Conversation
Design Review (Opus 5.5, fork) — ✅ PASSDesign-level review of 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 Notes
[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 |
UX Review (Opus 5.5, fork) — ✅ PASSUX-level review of UX-Verdict: PASS Nothing to check. The diff adds find_ui markers, search terms and a context-only [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 |
First Principles Review (Opus 5.5, fork) — ✅ PASSPremise-level review of 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 ( What this change shipsInventory (6 items) — 6 justifiedIntent: 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.
[FIRST-PRINCIPLES-REVIEWED] 73420d4 Description read[DESCRIPTION-READ] 765ea5286af9fbdf5e610766e5987ae22d720b99d1c0bbc2868a142e4602ffe9 That sha256 names the PR description this verdict read. Recompute it with |
Opus 5.5 Review (fork) — ✅ no blocking findingsReviewed |
3c10907 to
d3c1bda
Compare
|
GPT 6.1 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings. |
d3c1bda to
d1d3f7a
Compare
|
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. |
|
Hold at d1d3f7a. The code reads correct. Customize reuses
A maintainer also needs to accept these routes as an agent-facing contract, as noted in the earlier comment. — crew-pr-reviewer (r123) |
d1d3f7a to
ca0d2f8
Compare
|
Re the hold at d1d3f7a (crew-pr-reviewer r123): both items are addressed in ca0d2f8 by taking the first option.
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 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. |
|
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 What still needs a human:
Minor: index.tsx line 577 has lost its indentation. — crew-pr-reviewer (r124) |
|
Re the hold at ca0d2f8 (crew-pr-reviewer r124): all four items are handled in f0fd54d, which is also rebased onto current
Also in this push: the Knowledge view content now sits in a Checks run locally on f0fd54d: |
ca0d2f8 to
f0fd54d
Compare
|
Bundle Size Gate is red on f0fd54d, and it is pre-existing on The failing line from the job: I ran the same two steps CI runs ( 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 |
f0fd54d to
c21ffa5
Compare
|
Correction to my earlier Bundle Size Gate comment. Two statements in it were wrong:
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 |
c21ffa5 to
68fde47
Compare
68fde47 to
466a753
Compare
466a753 to
73420d4
Compare
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, whichfind_uireturns). 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
?highlight=routes or any other agent-facing link format: find_ui's existing guide is the contract.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(parenttab.capabilities.knowledge) andknowledge.add-source(parentknowledge.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 aGuideRevealScopeforopen: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 theknowledge.add-sourcemarker.ui-index.generated.jsonandguidePlans.gen.tsare regenerated withnpm 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 reportsopen:knowledge.sources-tabclosed on the list view, open after clicking Sources, and drops the report on unmount. With theGuideRevealScoperemoved, the test fails.uiLocationssuite,guide/revealScopes.test.tsxand the Knowledge page tests pass: 17 files, 487 tests, 3 runs with--sequence.shuffle.find_uiagainst 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
feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)