feat: graph management UI — admin page, picker, delete, member search#169
feat: graph management UI — admin page, picker, delete, member search#169charlie83Gs merged 12 commits intomainfrom
Conversation
… UX, ops (#162) Backend: - Add graph-scoped endpoints for all major resources (nodes sub-resources, facts, edges, sources, seeds, edge-candidates, conversations, syntheses) - Each resource gets a /graphs/{slug}/... router using GraphContext for session routing with proper RBAC checks - Add node_count to list_graphs via per-graph cross-schema count queries - Graph-scoped syntheses include graph_id in workflow dispatch payloads MCP: - Thread user_id through OAuth token claims (both OAuth access tokens and legacy API tokens) - Add GraphMember role verification in _get_graph_factory for non-default graphs — users must be members (or superusers) to access Frontend: - Add error/switching state to GraphProvider context - Show loading spinner in GraphPicker instead of hiding - Add active graph indicator (colored dot) for non-default graphs - Graceful 404 handling in graphRequest() with user-friendly message Operational: - Crash recovery: mark provisioning-stuck graphs as 'error' on API startup - Qdrant auto-recovery: ensure per-graph collections on worker startup - Both operations are idempotent and safe to run on every restart Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All 12 research endpoints now have graph-scoped versions at
/api/v1/graphs/{slug}/research/...:
- POST prepare (file upload + chunking)
- POST {conv_id}/confirm (dispatch ingest workflow)
- GET {conv_id}/sources (list ingest sources)
- GET {conv_id}/sources/{id}/download (download file)
- POST {conv_id}/decompose (dispatch decompose workflow)
- GET {conv_id}/proposals (fetch proposed nodes)
- POST {conv_id}/build (dispatch build workflow)
- POST bottom-up/prepare (dispatch bottom-up workflow)
- GET {conv_id}/bottom-up/proposals (fetch bottom-up results)
- POST {conv_id}/agent-select (dispatch agent selection)
- GET {conv_id}/agent-select/status (poll agent status)
- GET {conv_id}/summary (fetch research summary)
All workflow dispatches include graph_id in the payload. Write
endpoints enforce require_writer access control. Helpers are
reused from research.py to avoid duplication.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
) Code duplication (items 1 & 2): - Extract _impl functions from all 9 handler files (conversations, facts, edges, sources, seeds, edge_candidates, syntheses, nodes, research) — 38 _impl functions total - Graph-scoped files are now thin wrappers (~5 lines per endpoint) that import and call _impl functions - Net reduction: -1302 lines (from 2457 removed, 1155 added) N+1 in list_graphs (item 3): - Replace sequential per-graph node count queries with asyncio.gather for concurrent execution setTimeout race condition (item 4): - Replace arbitrary setTimeout(100ms) switching state with switchGeneration counter that consumers can key on MCP superuser round-trip (item 5): - Cache is_superuser flag in token claims at token creation time - _get_graph_factory skips membership DB query for superusers Ambiguous path params (item 6): - Change graph_edge_candidates pair endpoint from /{a:path}/{b:path} to /pair?seed_key_a=...&seed_key_b=... Qdrant recovery log level (item 7): - Bump from warning to error level for per-graph collection failures Missing require_writer (item 8): - Add require_writer to create_graph_synthesis and create_graph_super_synthesis Minor — 404 masking: - graphRequest() now only masks route-level 404s (no detail body), not resource-level 404s (which include "not found" in the message) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nd compat
The frontend calls /{seedKeyA}/{seedKeyB} path pattern for edge candidate
pairs. Keep graph-scoped endpoint consistent with the original to avoid
404s when graphRequest() routes to the graph-scoped version.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… masking, return type
Security — MCP fail-closed:
- _get_graph_factory now raises ValueError when token has no user_id
for non-default graphs instead of silently granting access
Bug — edge candidate path params:
- Add /pair query-param endpoint to original edge_candidates.py as
the unambiguous variant alongside the legacy path-param route
- Keep both graph-scoped and default /{a:path}/{b:path} for backward
compat since the frontend depends on this pattern
Fragile — 404 masking removed:
- All graph-scoped endpoints now exist, so the heuristic to distinguish
route-level vs resource-level 404s is no longer needed
- graphRequest() now passes errors through transparently
Minor — return type annotation:
- Add FileResponse return type to download_graph_ingest_source
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge conflict resolution: - research.py: take our _impl-extracted version (already includes graph_id propagation from PR #164) - uv.lock: regenerated Super-synthesis graph_id propagation (item 3): - Include graph_id in each sub_config dict so child synthesizer_wf dispatches target the correct graph Stale is_superuser claim (item 4): - Not actually stale — load_access_token and verify_token both re-query the User table on every request. Added clarifying comment. WriteSessionFactory type alias (item 7): - Replace Callable[..., "AsyncSession"] with proper async_sessionmaker[AsyncSession] type in edge_candidates.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test asserted session.get was called once, but our is_superuser caching now calls it twice (OAuthAccessToken + User). Update the mock to use side_effect=[row, user_mock] and assert call_count == 2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… member search (#168) - Fix graph-switch reactivity: all data-fetching hooks now refetch when active graph changes via switchGeneration - Fix createSynthesis/createSuperSynthesis to use graphRequest (was plain request) - Add "Graphs" link to sidebar admin nav section - Rewrite GraphPicker as Popover+Command org-style switcher with search and node counts - Add DeleteGraphDialog with "type slug to confirm" pattern - Wire delete into both list page (hover icon) and detail page (danger zone) - Create MemberSearch component — email autocomplete replaces UUID input - Add role description tooltips on member table header - Backend: add GET /graphs/database-connections endpoint (superuser) - Backend: add database_connection_name to GraphResponse - Graph creation form: DB connection picker when storage_mode=database - List page: search/filter by name + database, empty states, stats Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cy, double-fetch **Frontend endpoint graph wiring:** - seeds.synthesizePerspective/dismissPerspective/promote → graphRequest - export.nodes/facts/conversation → graphRequest - import.facts/nodes → graphRequest - streamImport helper → graph-aware URL via graphPath() - listGraphs/createGraph → plain request (not graph-scoped) **Backend review fixes:** - research.py: replace manual dict mutation (if graph_id: payload["graph_id"]) with dispatch_with_graph() across all 5 dispatch sites for consistency - qdrant_client: object → AsyncQdrantClient (TYPE_CHECKING) in edges.py, facts.py, nodes.py, seeds.py, export.py, import_service.py, dependencies.py - _recover_stuck_graphs: single UPDATE...RETURNING instead of SELECT-then-UPDATE race - MemberSearch: add error state with 403 handling for non-superuser graph admins - DB connection fetch loop: use loaded guard pattern to prevent infinite refetch - switchGeneration: include in useCallback deps (with eslint-disable) instead of separate useEffect to eliminate double-fetch on graph switch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
charlie83Gs
left a comment
There was a problem hiding this comment.
PR #169 Review
Nice feature set — the graph picker, delete dialog, and member search are well-built. A few things to address:
Blocking: Merge conflicts with main
The PR currently shows CONFLICTING merge status. Needs a rebase/merge from main. The conflicting files appear to be across both frontend and backend (GraphPicker.tsx, edges.py, facts.py, graphs.py, main.py, nodes.py, research.py, seeds.py).
Issues to fix
1. DB connection fetch can infinite-loop on error — frontend/src/app/graphs/page.tsx:50-58
When the listDatabaseConnections() call fails, the catch sets dbLoaded back to false, which changes the dependency array and re-triggers the effect, which fails again, ad infinitum. Either use a ref to track the attempt, or don't reset dbLoaded on failure (set an error state instead).
// Current (loops on error):
.catch(() => setDbLoaded(false));
// Fix: keep dbLoaded=true and track the error separately
.catch(() => setDbError("Failed to load connections"));2. MemberSearch: 403 error causes retry on every open — frontend/src/components/graphs/MemberSearch.tsx:47-55
When the API returns 403, loaded is never set to true, so every subsequent popover open retries the failing request. Set setLoaded(true) in the .finally() block or move it out of the .then().
// Fix: move setLoaded(true) to .finally()
.then((data) => {
setMembers(data);
})
.catch((err) => { ... })
.finally(() => {
setLoading(false);
setLoaded(true); // prevent retries on persistent errors
});3. Node count queries open N connections for N graphs — services/api/src/kt_api/graphs.py:199-214
_count_nodes resolves a new GraphSessions and opens a new DB session per graph via asyncio.gather. For a user with many graphs, this fans out to N concurrent DB connections on every list call. Consider capping concurrency with a semaphore, or storing node counts as a materialized counter (similar to NodeCounter in write-db) to avoid the fan-out.
Not blocking, but worth a TODO comment at minimum.
4. import asyncio inside function body — services/api/src/kt_api/graphs.py:193
Minor: move import asyncio to the top of the file with other stdlib imports.
Observations (non-blocking)
-
switchGenerationpattern: The eslint-disable comments withswitchGenerationin deps across 7+ hooks work correctly but are fragile. Consider extracting auseGraphScopedFetch(fetchFn, deps)hook that encapsulates this — would eliminate the repeated disable comments and make the pattern self-documenting. -
DeleteGraphDialog HTML entity: Line 67 uses
“— works fine in JSX but\u201c/\u201dis more idiomatic React. -
Access control looks solid:
_require_graph_accesswith role hierarchy,WITH FOR UPDATElocking for last-admin protection, superuser bypass — all well-implemented. The default graph open-read / superuser-write policy is clearly documented. -
Synthesis API fix:
createSynthesisandcreateSuperSynthesisnow correctly usegraphRequest— good catch and fix.
Summary
Fix #1 and #2 (both quick), resolve merge conflicts, and this is good to merge. #3 is worth tracking but not blocking.
…rt placement - DB connection fetch: use error state instead of resetting dbLoaded on failure (prevents infinite retry loop) - MemberSearch: set loaded=true in .finally() so 403 errors don't retry on every popover open - Node count queries: cap concurrency with asyncio.Semaphore(5) to avoid N concurrent DB connections for N graphs - Move `import asyncio` to top-level imports in graphs.py Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Replace require_admin with require_system_permission(SYSTEM_MANAGE_GRAPHS) on the GET /database-connections endpoint added in #169. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Implements #168 — comprehensive graph management UI improvements:
useNodeList,useEdgeList,useFactList,useSourceList,useSeedList,useGraphExplorer) now refetch when the active graph changes viaswitchGeneration. Pages (/investigate,/grow-graph) also react to switches.createSynthesis()andcreateSuperSynthesis()now usegraphRequest()instead of plainrequest(), ensuring syntheses are created in the correct graph.Layersicon to the admin section.<select>with aPopover+Commandswitcher — always visible, searchable, shows node counts per graph, with a "Manage graphs" link for superusers.DeleteGraphDialogwith "type slug to confirm" safety pattern. Wired into list page (hover icon) and detail page (danger zone section).MemberSearchcomponent replaces raw UUID input with email autocomplete usingGET /api/v1/members.GET /api/v1/graphs/database-connections(superuser-only) lists available database connections.database_connection_namefield, batch-fetched in list/detail endpoints.storage_mode=database.created_atdisplay,database_connection_nameon cards.database_connection_name,created_at, BYOK status, member count.Test plan
pnpm lint— passes cleanpnpm type-check— passes cleanpnpm test— 123/123 tests pass🤖 Generated with Claude Code