feat(us6): drop graph subsystem (PR-v5-6)#187
Merged
Merged
Conversation
BREAKING: removes internal/graph/* package and all FalkorDB integration.
- Delete `internal/graph/{noop,store,writer}.go` + `falkordb/{client,integration_test}.go`
- Delete `ui/src/views/GraphView.vue`
- Remove graph tool handlers from internal/mcp/server.go: graph_query, get_graph_stats, SetGraphStore
- Remove graph admin dispatch cases from internal/mcp/tools_admin.go
- Remove graphStore/graphWriter fields and FalkorDB init block from internal/worker/service.go
- Remove syncGraphFromRelations method + /api/graph/sync route + handleGraphSync handler
- Remove ExpandViaGraph + expansion constants + SetGraphStore from internal/search/manager.go
- Remove expandViaGraph hook field + expandGraphNeighbors helper from internal/worker/retrieval*.go
- Remove GraphStore field + FalkorDB edge creation from internal/synthesis/StoreEntitiesParams
- Remove ENGRAM_GRAPH_* + ENGRAM_FALKORDB_* + ENGRAM_INJECT_GRAPH_* env vars from internal/config/config.go
- `go mod tidy` drops falkordb-go v2.0.2 + redigo v1.9.3
FalkorDB Docker container RETAINED in docker-compose.yml per ADR-07 as inactive infra
for future v5.x graph-v2 reservation.
Build+vet+tests green (internal/mcp, internal/worker, internal/search, internal/synthesis, internal/config).
REVERSIBLE (code). IRREVERSIBLE (wiped FalkorDB data — per audit value=0).
Per Cross-Phase Conventions: C1 (no stubs, direct deletion), C7 (CodeRabbit only).
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughПолное удаление подсистемы графов: конфигурации, интерфейса/реализаций GraphStore (FalkorDB/Noop), асинхронного писателя, интеграции с worker/mcp/search/synthesis, UI-компонента GraphView и связанных маршрутов/тестов/зависимостей. Changes
Estimated code review effort🎯 4 (Сложная) | ⏱️ ~45 минут Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the entire graph subsystem, including the FalkorDB backend, graph-related configuration parameters, and graph-augmented search logic. It also cleans up associated MCP tools, administrative actions, and frontend components. Feedback suggests removing several stub methods—specifically handleGetGraphNeighbors, handleGetGraphStats, and lookupGraphSeedNeighbors—that were left behind and are no longer functional or referenced.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/mcp/tools_admin.go (1)
16-16: Сведите список admin-action к одному источнику правды.Сейчас перечень valid actions зашит строкой в двух ошибках здесь и ещё отдельно дублируется в описании
admin-tool вinternal/mcp/server.go. При следующем изменении они легко разъедутся. Лучше держать canonical[]string/map и генерировать из него и валидацию, и тексты ошибок, и metadata инструмента.Also applies to: 58-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcp/tools_admin.go` at line 16, The hard-coded list of valid admin actions is duplicated; introduce a single canonical variable (e.g., a package-level []string or map named validAdminActions or adminActionsSet) that contains all actions, use that variable in the validation logic in tools_admin.go (where the current error string is returned) to build the error message text dynamically, and also reference the same canonical variable when generating the admin tool metadata/description in internal/mcp/server.go so both validation and help text come from one source of truth.internal/worker/retrieval_helpers.go (1)
52-64: Добейте cleanup graph-пути в retrieval hooks.После удаления graph-expansion этот helper либо проксирует тестовый hook, либо всегда возвращает
nil. ПокаlookupGraphSeedNeighborsиretrievalHooks.getGraphNeighborsостаются в коде, висит ложный интеграционный контракт для уже удалённой подсистемы. Лучше удалить или переименовать их вместе, чтобы graph-сurface исчез полностью.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/retrieval_helpers.go` around lines 52 - 64, Удалите ложный graph-surface: удалите функцию lookupGraphSeedNeighbors и поле/hook retrievalHooks.getGraphNeighbors (или переименуйте его в явно тестовый символ, например getGraphNeighborsForTests) и уберите все вызовы normalizeObservationIDs(...) связанных с этим потоком; в коде ищите символы lookupGraphSeedNeighbors, retrievalHooks.getGraphNeighbors и normalizeObservationIDs, удалите/переименуйте определения и все места их использования (включая тест-хуки), чтобы полностью убрать оставшийся контракт graph-expansion из кода.internal/mcp/server.go (1)
4155-4165: Удалите legacy graph-handlers целиком или возвращайте обычную ошибку.Сейчас оба метода сериализуют успешный ответ с полем
"error": "graph backend removed". Если какой-то внутренний вызов ещё дойдёт сюда напрямую, это будет выглядеть как валидный payload, а не как удалённая возможность. После удаления registration/dispatch безопаснее убрать эти методы совсем либо вернуть нормальныйerrorиз handler'а.Also applies to: 4168-4177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/mcp/server.go` around lines 4155 - 4165, The handlers that currently build and return a serialized success payload (the local variable response containing "error": "graph backend removed" and using params.ObservationID / params.MaxHops) should not pretend to succeed; either remove these legacy graph-handler functions entirely or change them to return a real error instead of a JSON string payload (e.g., return an error from the handler or propagate fmt.Errorf("graph backend removed")). Apply the same change to the other similar block referenced (the one around the response map at the second occurrence) so callers cannot receive a valid-looking payload when the graph backend is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/mcp/server.go`:
- Around line 4155-4165: The handlers that currently build and return a
serialized success payload (the local variable response containing "error":
"graph backend removed" and using params.ObservationID / params.MaxHops) should
not pretend to succeed; either remove these legacy graph-handler functions
entirely or change them to return a real error instead of a JSON string payload
(e.g., return an error from the handler or propagate fmt.Errorf("graph backend
removed")). Apply the same change to the other similar block referenced (the one
around the response map at the second occurrence) so callers cannot receive a
valid-looking payload when the graph backend is removed.
In `@internal/mcp/tools_admin.go`:
- Line 16: The hard-coded list of valid admin actions is duplicated; introduce a
single canonical variable (e.g., a package-level []string or map named
validAdminActions or adminActionsSet) that contains all actions, use that
variable in the validation logic in tools_admin.go (where the current error
string is returned) to build the error message text dynamically, and also
reference the same canonical variable when generating the admin tool
metadata/description in internal/mcp/server.go so both validation and help text
come from one source of truth.
In `@internal/worker/retrieval_helpers.go`:
- Around line 52-64: Удалите ложный graph-surface: удалите функцию
lookupGraphSeedNeighbors и поле/hook retrievalHooks.getGraphNeighbors (или
переименуйте его в явно тестовый символ, например getGraphNeighborsForTests) и
уберите все вызовы normalizeObservationIDs(...) связанных с этим потоком; в коде
ищите символы lookupGraphSeedNeighbors, retrievalHooks.getGraphNeighbors и
normalizeObservationIDs, удалите/переименуйте определения и все места их
использования (включая тест-хуки), чтобы полностью убрать оставшийся контракт
graph-expansion из кода.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d0f8b4b-4352-4729-a8b5-250d262cb1f2
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
go.modinternal/config/config.gointernal/config/config_test.gointernal/graph/falkordb/client.gointernal/graph/falkordb/integration_test.gointernal/graph/noop.gointernal/graph/store.gointernal/graph/writer.gointernal/mcp/server.gointernal/mcp/tools_admin.gointernal/search/manager.gointernal/synthesis/entity_store.gointernal/worker/handlers_data.gointernal/worker/handlers_system.gointernal/worker/retrieval.gointernal/worker/retrieval_helpers.gointernal/worker/retrieval_test.gointernal/worker/service.goui/src/views/GraphView.vue
💤 Files with no reviewable changes (12)
- internal/config/config_test.go
- internal/worker/retrieval_test.go
- go.mod
- internal/worker/retrieval.go
- internal/worker/handlers_data.go
- internal/worker/service.go
- internal/graph/falkordb/integration_test.go
- internal/graph/writer.go
- internal/graph/noop.go
- ui/src/views/GraphView.vue
- internal/graph/store.go
- internal/graph/falkordb/client.go
UI build failed because src/router/index.ts still had 'component: () => import(@/views/GraphView.vue)' after GraphView.vue was deleted. Vite resolves lazy imports at build time. Also removed the 'Graph' entry from AppSidebar nav items. Fixes CI build failure on PR #187.
…ions - Remove lookupGraphSeedNeighbors stub + normalizeObservationIDs (retrieval_helpers.go) - Remove getGraphNeighbors field from retrievalHooks (retrieval.go) - Introduce adminActions []string single-source-of-truth (tools_admin.go) - Admin tool Description uses strings.Join(adminActions, ", ") (server.go) Build+vet+tests green.
thebtf
added a commit
that referenced
this pull request
Apr 18, 2026
Part of engram v5.0.0 US10 cleanup. Follows US6 (GraphView drop), US8-B (LearningView + AnalyticsView drop). - Delete `ui/src/views/PatternsView.vue` (21KB — references dropped patterns API from US5) - Delete `ui/src/views/MonitorView.vue` (15KB — observability view, replaced by server logs endpoint) - Delete `ui/src/views/SystemView.vue` (19KB — scoring stats, dropped in US8-C) - Remove router routes `/patterns`, `/monitor`, `/logs` (redirect), `/system` - Remove sidebar nav items: patterns, monitor, system Stats: 5 files changed, 1525 deletions. UI build (`npm run build`) green with surviving views. Companion PRs: - US6 (#187) ✅ — GraphView - US8-B (#188) ✅ — LearningView + AnalyticsView Stale API helpers in `ui/src/utils/api.ts` (patterns/monitor) retained — unused exports; will be cleaned in a dedicated UI cleanup PR. REVERSIBLE. Per C7 (CodeRabbit only).
thebtf
added a commit
that referenced
this pull request
Apr 19, 2026
…0) (#190) Part of engram v5.0.0 US10 cleanup. Follows US6 (GraphView drop), US8-B (LearningView + AnalyticsView drop). - Delete `ui/src/views/PatternsView.vue` (21KB — references dropped patterns API from US5) - Delete `ui/src/views/MonitorView.vue` (15KB — observability view, replaced by server logs endpoint) - Delete `ui/src/views/SystemView.vue` (19KB — scoring stats, dropped in US8-C) - Remove router routes `/patterns`, `/monitor`, `/logs` (redirect), `/system` - Remove sidebar nav items: patterns, monitor, system Stats: 5 files changed, 1525 deletions. UI build (`npm run build`) green with surviving views. Companion PRs: - US6 (#187) ✅ — GraphView - US8-B (#188) ✅ — LearningView + AnalyticsView Stale API helpers in `ui/src/utils/api.ts` (patterns/monitor) retained — unused exports; will be cleaned in a dedicated UI cleanup PR. REVERSIBLE. Per C7 (CodeRabbit only). Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
US6: Drop Graph Subsystem (BREAKING)
Part of engram v5.0.0 BREAKING cleanup — removes the
internal/graph/*package and all FalkorDB integration.Scope
internal/graph/{noop,store,writer}.go+falkordb/{client,integration_test}.go(5 files),ui/src/views/GraphView.vueinternal/mcp/server.go:graphpkgimport,graphStorefield,SetGraphStoremethod, tool registrationsgraph_query+get_graph_stats, dispatch cases (+ aliases),handleGetGraphStats,handleGraphQueryinternal/mcp/tools_admin.go: admin dispatch casesgraph+graph_statsinternal/worker/service.go: graphpkg/falkordb imports,graphStore/graphWriterfields, FalkorDB init block,syncGraphFromRelationsmethod,/api/graph/syncroute, wiring+shutdown blocksinternal/worker/handlers_data.go/handlers_system.go:handleGraphSynchandler, graph stats from system health,GraphStore:fromStoreEntitiesParams{}literalsinternal/search/manager.go:graphpkgimport,graphStorefield,SetGraphStoremethod,ExpandViaGraphmethod,graphExpansion*constantsinternal/worker/retrieval.go/retrieval_helpers.go:expandViaGraphhook field,expandGraphNeighborshelper + callerinternal/synthesis/entity_store.go:graphimport,GraphStorefield fromStoreEntitiesParams, FalkorDB edge creation blockinternal/config/config.go:ENGRAM_GRAPH_*,ENGRAM_FALKORDB_*,ENGRAM_INJECT_GRAPH_*env vars and struct fieldsDependencies removed by
go mod tidygithub.com/falkordb/falkordb-go v2.0.2+incompatiblegithub.com/gomodule/redigo v1.9.3(transitive through falkordb-go)FalkorDB Container RETAINED
Per ADR-07: docker-compose.yml
falkordbservice stays as inactive infrastructure reserved for v5.x graph-v2. Operator can wipe keyspace manually or leave it running.Stats
Context
Follows US5 (patterns drop, #186), US7 (consolidation, #185), US4 (project_settings, #184). Next: US8 (learning/scoring/LLM drop), US9 (search/expansion drop), US3-PR-B (observations table drop), US10-14.
Conventions Applied
git rm+ surgical edits; NO//go:build ignoreReversibility
git revert🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor