Skip to content

feat(us5): drop patterns subsystem (PR-v5-5)#186

Merged
thebtf merged 2 commits into
mainfrom
feat/v5-us5-patterns
Apr 18, 2026
Merged

feat(us5): drop patterns subsystem (PR-v5-5)#186
thebtf merged 2 commits into
mainfrom
feat/v5-us5-patterns

Conversation

@thebtf
Copy link
Copy Markdown
Owner

@thebtf thebtf commented Apr 18, 2026

US5: Drop Patterns Subsystem (BREAKING)

Part of engram v5.0.0 BREAKING cleanup — removes the patterns and pattern_observations tables and all related code.

Scope

  • DROP tables: patterns, pattern_observations (CASCADE) via migration 098_drop_patterns_and_pattern_observations
  • DELETE packages: internal/pattern/ (detector, classifier, models, store) — 4 files
  • DELETE files: internal/db/gorm/pattern_store.go, internal/worker/handlers_patterns.go
  • REMOVE from internal/worker/service.go: pattern import, patternStore + patternDetector fields, init/setup/shutdown blocks
  • REMOVE from internal/mcp/server.go: patternStore field, NewServer param, handleGetPatterns, pattern stats from handleGetMemoryStats, pattern health from handleSystemHealth, get_patterns dispatch + tool registration
  • REMOVE from internal/db/gorm/models.go: Pattern struct + TableName + BeforeCreate hook
  • UPDATE tests: NewServer arity, pattern-specific tests removed, regression assertions added

Stats

  • 16 files changed, 969 insertions(+), 4578 deletions(-)
  • Build + vet + tests all green

Context

Follows US4 (project_settings drop, #184) and US7 (consolidation + maintenance + reranking + smart_gc, #185). Next: US6 (graph), US8 (learning), US9 (search/expansion).

Conventions Applied

  • C1 (no stubs): direct git rm for obsolete files, surgical edits on callers
  • C3 (migration IDs): 098 assigned (US4=097, US7 no migration)
  • C7 (CodeRabbit only): review via CodeRabbit, no Gemini

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Удаленные функции

    • Удалена подсистема обнаружения и анализа паттернов (детектор, качество, продвижение кандидатов).
    • Удалены REST-эндпоинты и интерфейсы для управления паттернами и получения их статистики/insights.
    • Убран инструмент/действие получения паттернов из MCP/recall.
  • База данных

    • Добавлена миграция, удаляющая таблицы паттернов и связанных наблюдений.

Implements plan.md §Phase 5 — delete internal/pattern/ package + patterns tables + all callers.

Deleted (7 files):
- internal/pattern/ (4 files: decay.go, detector.go, detector_test.go, quality.go)
- internal/db/gorm/pattern_store.go + pattern_store_test.go
- internal/worker/handlers_patterns.go

Caller cleanup:
- internal/worker/service.go: remove pattern import, patternStore + patternDetector struct fields, NewPatternStore init, NewDetector init, setupCallbacks pattern body, pattern_detector.Start() + Stop() shutdown
- internal/mcp/server.go: remove pattern imports, field, tool registrations, dispatch cases, handler methods
- internal/mcp/tools_recall.go + tools_learning_test.go: remove pattern references
- internal/mcp/server_test.go: NewServer call arity updated, pattern-related test cases removed
- internal/db/gorm/models.go: remove Pattern + PatternObservation structs
- internal/db/gorm/migrations.go: add 098_drop_patterns_and_pattern_observations (C3-compliant rollback)
- internal/db/gorm/benchmark_test.go + integration_test.go: remove pattern-specific test cases

Verified:
  go build ./... clean
  go vet ./... clean
  go test -short -count=1 ./... all suites green

Per plan.md conventions:
  C3: migration 098 rollback returns IRREVERSIBLE error
  C6: DROP TABLE IF EXISTS CASCADE is portable
  C7: CodeRabbit-only review
  C8: single coherent PR

Plan.md §Phase 5. Unblocked after US7 merged (maintenance consumer gone).
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

Walkthrough

Этот пул-реквест полностью удаляет подсистему распознавания паттернов: модели, хранилище, детектор, оценочные функции, HTTP-обработчики, интеграцию MCP, связанные тесты и добавляет миграцию для удаления таблиц patterns и pattern_observations.

Changes

Cohort / File(s) Summary
Миграции БД
internal/db/gorm/migrations.go
Заменён AutoMigrate для patterns на явный DDL, реформатированы блоки миграций; добавлена миграция 098_drop_patterns, удаляющая pattern_observations и patterns (rollback — irreversible).
GORM-модели и хранилище
internal/db/gorm/models.go, internal/db/gorm/pattern_store.go
Удалён тип Pattern, его методы и весь файл pattern_store.go — убраны PatternStore, PatternCleanupFunc и все методы для CRUD, поиска, объединения и статистики по паттернам.
Тесты и бенчмарки БД
internal/db/gorm/benchmark_test.go, internal/db/gorm/integration_test.go, internal/db/gorm/pattern_store_test.go, internal/db/gorm/migrations_integration_test.go
Удалён бенчмарк BenchmarkPatternStore_StorePattern и ненужный импорт database/sql. Из интеграционных тестов убрана логика создания/проверки паттернов; удалён файл с тестами pattern_store_test.go. Добавлен тест TestMigrationsIntegration_PatternsDropped.
Пакет pattern (логика)
internal/pattern/decay.go, internal/pattern/detector.go, internal/pattern/quality.go, internal/pattern/detector_test.go
Полностью удалены: RunDecay, детектор (включая Detector, DetectionResult, config, методы Start/Stop/Analyze и пр.), функции качества (QualityScore, TemporalDecay, DynamicQuality) и все связанные тесты/хелперы.
Worker — обработчики и сервис
internal/worker/handlers_patterns.go, internal/worker/service.go
Удалены все REST-обработчики для /api/patterns*, типы запросов/ответов, DefaultPatternsLimit; из Service удалены поля patternStore и patternDetector, их инициализация, lifecycle и роуты.
MCP — сервер и инструменты
internal/mcp/server.go, internal/mcp/tools_recall.go, internal/mcp/server_test.go, internal/mcp/tools_learning_test.go
Конструктор NewServer пересобран на ServerOptions (убран patternStore); удалён инструмент get_patterns и соответствующий handler; удалено действие patterns в recall; обновлены тесты (удалён тест валидации для get_patterns, обновлён вызов NewServer).

Sequence Diagram(s)

(Пропущено — изменения удаляют подсистему и не вводят новую последовательность взаимодействий.)

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested labels

architecture, breaking-change, database-migration, refactoring

Poem

🐰 Я — кролик, прыг, и шепчу в тиши,
Паттерны ушли — и стало чище в пути.
Миграции роняют их следы,
Код полегчал, отключились беды.
Пусть ветки вернутся к новым мечтам — — ура!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Название PR точно отражает основное изменение: удаление подсистемы patterns из проекта engram v5.0.0.
Docstring Coverage ✅ Passed Docstring coverage is 92.98% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/v5-us5-patterns

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Comment @coderabbitai help to get the list of available commands and usage tips.

@thebtf
Copy link
Copy Markdown
Owner Author

thebtf commented Apr 18, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes the pattern detection subsystem, including the PatternStore, Detector, and associated models, benchmarks, and API routes. It also introduces a migration to drop the patterns and pattern_observations tables. Feedback indicates that the raw SQL replacement for the initial pattern table migration is incomplete, missing several columns required for the migration history to remain runnable. Furthermore, the creation migration for the pattern_observations table must be updated to raw SQL to prevent failures on new installations following the deletion of the underlying struct. Finally, there is a recommendation to synchronize the recall tool's description, action enum, and error messages to address existing discrepancies.

Comment on lines +249 to +264
sql := `CREATE TABLE IF NOT EXISTS patterns (
id BIGSERIAL PRIMARY KEY,
name TEXT NOT NULL,
type TEXT NOT NULL,
project TEXT,
description TEXT,
recommendation TEXT,
frequency INTEGER NOT NULL DEFAULT 1,
confidence REAL NOT NULL DEFAULT 0.5,
status TEXT NOT NULL DEFAULT 'active',
created_at TEXT NOT NULL DEFAULT NOW()::text,
created_at_epoch BIGINT NOT NULL DEFAULT 0,
last_seen_at TEXT NOT NULL DEFAULT NOW()::text,
last_seen_at_epoch BIGINT NOT NULL DEFAULT 0
)`
return tx.Exec(sql).Error
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The raw SQL replacement for AutoMigrate(&Pattern{}) in migration 009_patterns is incomplete. It is missing several columns that were present in the original Pattern struct: signature, projects (misspelled as project in the current diff), observation_ids, and merged_into_id. Even though the table is eventually dropped in migration 098, the migration history must remain runnable from scratch. If any intermediate migrations rely on these columns, they will fail for new installations.

				sql := `CREATE TABLE IF NOT EXISTS patterns (\n					id BIGSERIAL PRIMARY KEY,\n					name TEXT NOT NULL,\n					type TEXT NOT NULL,\n					projects TEXT,\n					signature TEXT,\n					description TEXT,\n					recommendation TEXT,\n					frequency INTEGER NOT NULL DEFAULT 1,\n					confidence REAL NOT NULL DEFAULT 0.5,\n					status TEXT NOT NULL DEFAULT 'active',\n					observation_ids TEXT,\n					merged_into_id BIGINT,\n					created_at TEXT NOT NULL DEFAULT NOW()::text,\n					created_at_epoch BIGINT NOT NULL DEFAULT 0,\n					last_seen_at TEXT NOT NULL DEFAULT NOW()::text,\n					last_seen_at_epoch BIGINT NOT NULL DEFAULT 0\n				)`\n				return tx.Exec(sql).Error

{
ID: "098_drop_patterns",
Migrate: func(tx *gorm.DB) error {
if err := tx.Exec(`DROP TABLE IF EXISTS pattern_observations CASCADE`).Error; err != nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The PR description mentions dropping the pattern_observations table, but its creation migration is not updated to raw SQL in this PR. If this table was originally created via AutoMigrate of a struct that has now been deleted, the migration history will break for new installations. Please ensure that the migration which created pattern_observations is also updated to use raw SQL.

Comment thread internal/mcp/server.go
Comment on lines +485 to +490
Description: "Search and retrieve memories. Actions: search (default), preset (decisions/changes/how_it_works), by_file, by_concept, by_type, similar, timeline, related, get, sessions, explain, reasoning.",
tier: tierCore,
InputSchema: map[string]any{
"type": "object",
"properties": map[string]any{
"action": map[string]any{"type": "string", "enum": []string{"search", "preset", "by_file", "by_concept", "by_type", "similar", "timeline", "related", "patterns", "get", "sessions", "explain", "reasoning", "hit_rate"}, "default": "search", "description": "Action to perform"},
"action": map[string]any{"type": "string", "enum": []string{"search", "preset", "by_file", "by_concept", "by_type", "similar", "timeline", "related", "get", "sessions", "explain", "reasoning", "hit_rate"}, "default": "search", "description": "Action to perform"},
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There is a discrepancy between the recall tool's description and its action enum. hit_rate is present in the enum but missing from the description. Additionally, the error message in internal/mcp/tools_recall.go lists wake_up, taxonomy, and tunnels as valid actions, but they are missing from both the enum and the description here. Since this line is being modified to remove patterns, it's a good opportunity to synchronize these fields.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
internal/worker/service.go (1)

513-513: Упростите API setupCallbacks после удаления pattern-hook.

Сейчас вызов всё ещё прокидывает observationStore и processor, хотя колбэки их больше не используют. Лучше убрать эти параметры из сигнатуры и вызова, чтобы не оставлять ложный wiring.

♻️ Предлагаемое упрощение
-func (s *Service) setupCallbacks(
-	observationStore *gorm.ObservationStore,
-	processor *sdk.Processor,
-	sessionManager *session.Manager,
-) {
+func (s *Service) setupCallbacks(sessionManager *session.Manager) {
-	s.setupCallbacks(observationStore, processor, sessionManager)
+	s.setupCallbacks(sessionManager)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/worker/service.go` at line 513, The call to setupCallbacks still
passes observationStore and processor even though the callbacks no longer use
them; remove those parameters from both the setupCallbacks function signature
and its call (leave sessionManager or other remaining args like
s.setupCallbacks(sessionManager) as needed), update the setupCallbacks
implementation to stop referencing observationStore and processor, and search
for/adjust any other call sites or interface declarations that accept those two
parameters so the codebase compiles cleanly.
internal/db/gorm/migrations.go (1)

249-264: Добавьте регрессионный тест на fresh-install цепочку миграций.

Теперь корректность новой установки зависит от вручную поддерживаемого DDL удалённой подсистемы в 009_patterns и его корректного сноса в 098_drop_patterns. Без отдельного теста легко сломать bootstrap пустой БД так, что апгрейды со старых инсталляций останутся зелёными.

Also applies to: 3134-3146

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/db/gorm/migrations.go` around lines 249 - 264, Add a regression test
that validates running the full migration chain on a fresh/empty database so new
installs don't break due to the DDL in the 009_patterns migration and its
teardown in 098_drop_patterns; implement a test (in the same package as
internal/db/gorm migrations tests) which creates a temporary empty DB, runs all
migrations from start to finish (invoking the same migration runner used in
migrations.go), asserts the migration run returns no error, and verifies the
expected end-state (e.g., that the patterns table was created then dropped
according to 009_patterns and 098_drop_patterns or that the final schema matches
the latest expected tables), referencing the migration identifiers 009_patterns
and 098_drop_patterns to ensure both are covered.
internal/mcp/server.go (1)

59-71: NewServer уже слишком хрупкий для позиционного конструктора.

Удаление одного поля снова сдвинуло все вызовы конструктора. Сигнатура с 11 зависимостями повышает шанс тихо перепутать аргументы в следующих PR; тут уже выгоднее перейти на config/options struct и перестать таскать такие массовые правки по callsite-ам.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/mcp/server.go` around lines 59 - 71, NewServer currently takes 11
positional dependencies which is fragile; change it to accept a single
config/options struct (e.g., type ServerOptions struct { SearchMgr
*search.Manager; Version string; ObservationStore *gorm.ObservationStore;
RelationStore *gorm.RelationStore; SessionStore *gorm.SessionStore;
ScoreCalculator *scoring.Calculator; Recalculator *scoring.Recalculator;
CollectionRegistry *collections.Registry; SessionIdxStore *sessions.Store;
DocumentStore *gorm.DocumentStore; ChunkManager *chunking.Manager }) and update
NewServer to accept (opts ServerOptions) or *(opts *ServerOptions) and read
fields from opts instead of positional args, then update all NewServer call
sites to construct and pass that struct (using field names so order doesn’t
matter). Optionally provide defaults/validation inside NewServer (e.g., check
nil required fields) to keep runtime errors obvious; reference NewServer and
ServerOptions when applying changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/mcp/server.go`:
- Around line 485-490: The initialize.instructions response still advertises the
removed "patterns" recall.action; update engramInstructions (the
initialize.instructions payload) to remove "patterns" from the recall/action
enum so it matches the actual recall.action implementation, ensuring MCP clients
are not told to call the deleted "patterns" action; locate the enum/list inside
engramInstructions and remove "patterns" (or replace with the correct current
actions) so initialize.instructions and recall.action stay in sync.

---

Nitpick comments:
In `@internal/db/gorm/migrations.go`:
- Around line 249-264: Add a regression test that validates running the full
migration chain on a fresh/empty database so new installs don't break due to the
DDL in the 009_patterns migration and its teardown in 098_drop_patterns;
implement a test (in the same package as internal/db/gorm migrations tests)
which creates a temporary empty DB, runs all migrations from start to finish
(invoking the same migration runner used in migrations.go), asserts the
migration run returns no error, and verifies the expected end-state (e.g., that
the patterns table was created then dropped according to 009_patterns and
098_drop_patterns or that the final schema matches the latest expected tables),
referencing the migration identifiers 009_patterns and 098_drop_patterns to
ensure both are covered.

In `@internal/mcp/server.go`:
- Around line 59-71: NewServer currently takes 11 positional dependencies which
is fragile; change it to accept a single config/options struct (e.g., type
ServerOptions struct { SearchMgr *search.Manager; Version string;
ObservationStore *gorm.ObservationStore; RelationStore *gorm.RelationStore;
SessionStore *gorm.SessionStore; ScoreCalculator *scoring.Calculator;
Recalculator *scoring.Recalculator; CollectionRegistry *collections.Registry;
SessionIdxStore *sessions.Store; DocumentStore *gorm.DocumentStore; ChunkManager
*chunking.Manager }) and update NewServer to accept (opts ServerOptions) or
*(opts *ServerOptions) and read fields from opts instead of positional args,
then update all NewServer call sites to construct and pass that struct (using
field names so order doesn’t matter). Optionally provide defaults/validation
inside NewServer (e.g., check nil required fields) to keep runtime errors
obvious; reference NewServer and ServerOptions when applying changes.

In `@internal/worker/service.go`:
- Line 513: The call to setupCallbacks still passes observationStore and
processor even though the callbacks no longer use them; remove those parameters
from both the setupCallbacks function signature and its call (leave
sessionManager or other remaining args like s.setupCallbacks(sessionManager) as
needed), update the setupCallbacks implementation to stop referencing
observationStore and processor, and search for/adjust any other call sites or
interface declarations that accept those two parameters so the codebase compiles
cleanly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4553e398-2045-40e1-a1cb-155491329b81

📥 Commits

Reviewing files that changed from the base of the PR and between 25b1f86 and 092a704.

📒 Files selected for processing (16)
  • internal/db/gorm/benchmark_test.go
  • internal/db/gorm/integration_test.go
  • internal/db/gorm/migrations.go
  • internal/db/gorm/models.go
  • internal/db/gorm/pattern_store.go
  • internal/db/gorm/pattern_store_test.go
  • internal/mcp/server.go
  • internal/mcp/server_test.go
  • internal/mcp/tools_learning_test.go
  • internal/mcp/tools_recall.go
  • internal/pattern/decay.go
  • internal/pattern/detector.go
  • internal/pattern/detector_test.go
  • internal/pattern/quality.go
  • internal/worker/handlers_patterns.go
  • internal/worker/service.go
💤 Files with no reviewable changes (8)
  • internal/db/gorm/benchmark_test.go
  • internal/pattern/quality.go
  • internal/pattern/decay.go
  • internal/db/gorm/pattern_store_test.go
  • internal/pattern/detector_test.go
  • internal/pattern/detector.go
  • internal/worker/handlers_patterns.go
  • internal/db/gorm/pattern_store.go

Comment thread internal/mcp/server.go
- Remove stale `patterns` from engramInstructions recall.action list (MAJOR)
- Simplify setupCallbacks signature — remove unused observationStore/processor params
- Refactor NewServer to accept ServerOptions struct (11 positional -> named fields)
- Add TestMigrationsIntegration_PatternsDropped regression test for 009/098 chain

Reviewer comment ids: PRRT_kwDORY-ek85799yU, coderabbit-nitpick-4efeac87-513,
coderabbit-nitpick-3c2c00b3-59, coderabbit-nitpick-0b459624-249

Build+vet+test green (internal/mcp, internal/worker, internal/db/gorm).
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/mcp/server.go (1)

829-831: ⚠️ Potential issue | 🟡 Minor

Описание инструмента содержит устаревшую ссылку на pattern counts.

После удаления подсистемы паттернов описание get_memory_stats всё ещё упоминает "pattern counts", хотя эта метрика больше не возвращается функцией handleGetMemoryStats.

🔧 Предлагаемое исправление
 		{
 			Name:        "get_memory_stats",
-			Description: "Get statistics about the memory system including observation counts, vector stats, pattern counts, and search metrics.",
+			Description: "Get statistics about the memory system including observation counts and search metrics.",
 			tier:        tierAdmin,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/mcp/server.go` around lines 829 - 831, Описание инструмента
get_memory_stats устарело — оно по-прежнему упоминает "pattern counts", хотя
handleGetMemoryStats больше их не возвращает; обновите поле Description в
структуре с Name: "get_memory_stats" чтобы удалить упоминание "pattern counts" и
при необходимости кратко перечислить только актуальные метрики (observation
counts, vector stats, search metrics), оставляя текст согласованным с
реализацией handleGetMemoryStats.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/db/gorm/migrations_integration_test.go`:
- Around line 84-87: The test calls runMigrations(db) but may pass if the DB is
already migrated; modify the test in migrations_integration_test.go to ensure a
fresh-install path by resetting migration state before invoking runMigrations:
either recreate the test database/connection, drop all tables, or clear the
migrations tracking table (whichever your migration framework uses) for the db
variable so that 009_patterns and 098_drop_patterns will actually run; ensure
this reset step is performed immediately before calling runMigrations(db) and is
part of the test teardown to avoid leaking state to other tests.

In `@internal/mcp/server.go`:
- Around line 392-395: В таблице действий для `recall` отсутствует действие
`hit_rate`, хотя оно определено в `InputSchema` — добавьте `hit_rate` в список
действий в таблице-инструкций рядом с `search`, `preset`, и т.д.; откройте
функцию/const/variable that renders the markdown table in internal/mcp/server.go
(the block containing the `| Tool | Purpose | Key Actions |` table) and insert
`hit_rate` into the `recall` row's Key Actions cell so the documentation matches
the schema (`InputSchema`/`hit_rate`).

---

Outside diff comments:
In `@internal/mcp/server.go`:
- Around line 829-831: Описание инструмента get_memory_stats устарело — оно
по-прежнему упоминает "pattern counts", хотя handleGetMemoryStats больше их не
возвращает; обновите поле Description в структуре с Name: "get_memory_stats"
чтобы удалить упоминание "pattern counts" и при необходимости кратко перечислить
только актуальные метрики (observation counts, vector stats, search metrics),
оставляя текст согласованным с реализацией handleGetMemoryStats.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fb402317-9f21-497a-bd2a-5a5971804cf3

📥 Commits

Reviewing files that changed from the base of the PR and between 092a704 and 23c82db.

📒 Files selected for processing (5)
  • internal/db/gorm/migrations_integration_test.go
  • internal/mcp/server.go
  • internal/mcp/server_test.go
  • internal/mcp/tools_learning_test.go
  • internal/worker/service.go
🚧 Files skipped from review as they are similar to previous changes (3)
  • internal/mcp/tools_learning_test.go
  • internal/mcp/server_test.go
  • internal/worker/service.go

Comment on lines +84 to +87
// Run the full migration chain — this covers both 009_patterns (create) and
// 098_drop_patterns (drop), exercising the fresh-install path end-to-end.
require.NoError(t, runMigrations(db), "full migration chain must succeed on fresh DB")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Тест может ложно проходить, не проверяя fresh-install путь.

На Line 84-87 вызывается runMigrations(db) без принудительного сброса состояния для 009_patterns/098_drop_patterns. Если БД уже мигрирована, эти шаги не выполнятся, и регрессию можно пропустить.

Предлагаемое исправление
-	// Run the full migration chain — this covers both 009_patterns (create) and
-	// 098_drop_patterns (drop), exercising the fresh-install path end-to-end.
-	require.NoError(t, runMigrations(db), "full migration chain must succeed on fresh DB")
+	// Force re-execution of the two migrations under test, so this test is stable
+	// even when DATABASE_DSN points to a previously migrated database.
+	require.NoError(t, db.Exec(`DELETE FROM migrations WHERE id IN (?, ?)`, "009_patterns", "098_drop_patterns").Error)
+	require.NoError(t, db.Exec(`DROP TABLE IF EXISTS pattern_observations CASCADE`).Error)
+	require.NoError(t, db.Exec(`DROP TABLE IF EXISTS patterns CASCADE`).Error)
+	require.NoError(t, runMigrations(db), "migration chain must re-run 009_patterns and 098_drop_patterns")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Run the full migration chain — this covers both 009_patterns (create) and
// 098_drop_patterns (drop), exercising the fresh-install path end-to-end.
require.NoError(t, runMigrations(db), "full migration chain must succeed on fresh DB")
// Force re-execution of the two migrations under test, so this test is stable
// even when DATABASE_DSN points to a previously migrated database.
require.NoError(t, db.Exec(`DELETE FROM migrations WHERE id IN (?, ?)`, "009_patterns", "098_drop_patterns").Error)
require.NoError(t, db.Exec(`DROP TABLE IF EXISTS pattern_observations CASCADE`).Error)
require.NoError(t, db.Exec(`DROP TABLE IF EXISTS patterns CASCADE`).Error)
require.NoError(t, runMigrations(db), "migration chain must re-run 009_patterns and 098_drop_patterns")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/db/gorm/migrations_integration_test.go` around lines 84 - 87, The
test calls runMigrations(db) but may pass if the DB is already migrated; modify
the test in migrations_integration_test.go to ensure a fresh-install path by
resetting migration state before invoking runMigrations: either recreate the
test database/connection, drop all tables, or clear the migrations tracking
table (whichever your migration framework uses) for the db variable so that
009_patterns and 098_drop_patterns will actually run; ensure this reset step is
performed immediately before calling runMigrations(db) and is part of the test
teardown to avoid leaking state to other tests.

Comment thread internal/mcp/server.go
Comment on lines 392 to 395
| Tool | Purpose | Key Actions |
|------|---------|-------------|
| ` + "`recall`" + ` | **Search & retrieve** memories | search (default), preset, by_file, by_concept, by_type, similar, timeline, related, patterns, get, sessions, explain, reasoning |
| ` + "`recall`" + ` | **Search & retrieve** memories | search (default), preset, by_file, by_concept, by_type, similar, timeline, related, get, sessions, explain, reasoning |
| ` + "`store`" + ` | **Save** memories, edit, merge, extract | create (default), edit, merge, import, extract |
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Действие hit_rate отсутствует в документации.

В таблице инструкций (Line 394) перечислены доступные действия для recall, но hit_rate не указан, хотя он присутствует в схеме InputSchema (Line 493). MCP-клиенты, использующие эту документацию, не узнают о существовании данного действия.

🔧 Предлагаемое исправление
-| ` + "`recall`" + ` | **Search & retrieve** memories | search (default), preset, by_file, by_concept, by_type, similar, timeline, related, get, sessions, explain, reasoning |
+| ` + "`recall`" + ` | **Search & retrieve** memories | search (default), preset, by_file, by_concept, by_type, similar, timeline, related, get, sessions, explain, reasoning, hit_rate |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/mcp/server.go` around lines 392 - 395, В таблице действий для
`recall` отсутствует действие `hit_rate`, хотя оно определено в `InputSchema` —
добавьте `hit_rate` в список действий в таблице-инструкций рядом с `search`,
`preset`, и т.д.; откройте функцию/const/variable that renders the markdown
table in internal/mcp/server.go (the block containing the `| Tool | Purpose |
Key Actions |` table) and insert `hit_rate` into the `recall` row's Key Actions
cell so the documentation matches the schema (`InputSchema`/`hit_rate`).

@thebtf thebtf merged commit 7afd33f into main Apr 18, 2026
8 checks passed
@thebtf thebtf deleted the feat/v5-us5-patterns branch April 18, 2026 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant