feat: tech debt sprint — sessions view + sidebar + DB cleanup#42
Conversation
Backend: - Add ListSDKSessions with pagination + project filter - Add GET /api/sessions/list endpoint - Register route before /api/sessions (specificity) Frontend: - Sessions page now queries SDK sessions, not indexed transcripts - Info sidebar (health, memory, retrieval) moved into AppSidebar - Remove old Sidebar from HomeView — timeline gets full width
- Migration 047: DROP TABLE IF EXISTS memory_blocks (unused since creation) - Add DeleteOrphanedCredentials method (by fingerprint mismatch) - Add DELETE /api/vault/orphaned-credentials endpoint
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPR добавляет миграцию для удаления таблицы Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Service as Service (worker)
participant Vault as Vault
participant ObservationStore as ObservationStore
participant DB as Database
Client->>Service: DELETE /api/vault/orphaned-credentials
Service->>Vault: getVault()
alt fingerprint available
Service->>ObservationStore: DeleteOrphanedCredentials(ctx, fingerprint)
ObservationStore->>DB: DELETE FROM observations WHERE type='credential' AND encrypted_secret IS NOT NULL AND encryption_key_fingerprint IS NOT NULL AND encryption_key_fingerprint != fingerprint
DB-->>ObservationStore: RowsAffected
ObservationStore-->>Service: (rows, nil)
Service-->>Client: 200 {"status":"ok","deleted":rows}
else missing fingerprint / service not ready
Service-->>Client: 503 {"error": "..."}
end
sequenceDiagram
participant Client as Client
participant Service as Service (worker)
participant SessionStore as SessionStore
participant DB as Database
Client->>Service: GET /api/sessions/list?project=&limit=&offset=
Service->>SessionStore: ListSDKSessions(ctx, project, limit, offset)
SessionStore->>DB: COUNT ... WHERE project = ? (optional)
DB-->>SessionStore: total
SessionStore->>DB: SELECT ... ORDER BY started_at_epoch DESC, id DESC LIMIT ? OFFSET ?
DB-->>SessionStore: rows
SessionStore-->>Service: (sessions, total, nil)
Service-->>Client: 200 {"sessions":[...],"total":total,"limit":...,"offset":...}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses several technical debt items, significantly improving the application's session management, user interface layout, and backend data hygiene. The changes streamline how session data is retrieved and displayed, optimize the main view by integrating system information into the navigation sidebar, and enhance database integrity by removing unused tables and providing a mechanism to clean up unrecoverable encrypted credentials. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
@coderabbitai review |
|
@gemini-code-assist review |
✅ Actions performedReview triggered.
|
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable features and cleanups, including a new sessions view with a corresponding API, an endpoint for cleaning up orphaned credentials, and a database migration to remove an unused table. The frontend refactoring to relocate the info sidebar is also a good improvement. The code is generally well-structured and follows good practices. I have one suggestion to improve type safety in the useSessions composable by avoiding an as any type cast.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/worker/handlers_sessions.go (1)
421-429: Лучше возвращать 400 для невалидныхlimit/offset, а не молча подставлять дефолт.Сейчас ошибки параметров скрываются, из-за чего клиенту сложно понять причину «странной» пагинации.
💡 Предлагаемое исправление
if v := r.URL.Query().Get("limit"); v != "" { - if parsed, err := strconv.Atoi(v); err == nil && parsed > 0 && parsed <= 200 { - limit = parsed - } + parsed, err := strconv.Atoi(v) + if err != nil || parsed <= 0 || parsed > 200 { + http.Error(w, "invalid limit: must be 1..200", http.StatusBadRequest) + return + } + limit = parsed } if v := r.URL.Query().Get("offset"); v != "" { - if parsed, err := strconv.Atoi(v); err == nil && parsed >= 0 { - offset = parsed - } + parsed, err := strconv.Atoi(v) + if err != nil || parsed < 0 { + http.Error(w, "invalid offset: must be >= 0", http.StatusBadRequest) + return + } + offset = parsed }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_sessions.go` around lines 421 - 429, The handler currently swallows invalid limit/offset query values and falls back to defaults; update the parsing in the sessions listing handler so that when r.URL.Query().Get("limit") or "offset" is present but fails validation (non-integer, limit <=0 or >200, or offset <0) the handler returns a 400 response instead of using defaults. Locate the parsing block around limit/offset in internal/worker/handlers_sessions.go (the code using strconv.Atoi and variables limit/offset) and change it to validate presence+value, and on invalid input call the existing HTTP error response path (e.g., http.Error or the handler's standard JSON error responder) with a clear message about the bad parameter and stop further processing.ui/src/composables/useSessions.ts (3)
20-21: Неиспользуемые реактивные переменныеfilterFromиfilterTo.После перехода на
fetchSDKSessionsэти фильтры больше не передаются в API (строки 31-38), но продолжают возвращаться из composable (строка 109). Это мёртвый код, который может ввести в заблуждение.Если фильтрация по дате планируется в будущем, стоит добавить TODO-комментарий. Иначе рекомендуется удалить эти переменные.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useSessions.ts` around lines 20 - 21, Remove the dead reactive variables filterFrom and filterTo from useSessions.ts (delete their declarations and remove them from the returned object) because fetchSDKSessions no longer uses them; if date filtering is intended later, replace removal with a single TODO comment near fetchSDKSessions and keep only the TODOed placeholders, but do not return unused refs from the composable.
34-35: Жёстко заданная пагинация без возможности загрузки следующих страниц.
limit: 50иoffset: 0зафиксированы. Если у пользователя более 50 сессий, остальные будут недоступны. Стоит предусмотреть механизм подгрузки (infinite scroll или кнопка "Load more").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useSessions.ts` around lines 34 - 35, В текущем useSessions композиционном файле жёстко зафиксированы параметры пагинации (limit: 50, offset: 0) — это блокирует доступ к сессиям за пределами первых 50. Измените реализацию в useSessions так, чтобы limit был конфигурируемым (по умолчанию 50) и offset — реактивным состоянием, и добавьте метод (например loadMore или fetchMoreSessions) который увеличивает offset и вызывает существующую функцию загрузки (например fetchSessions или getSessions) для подгрузки следующей страницы; альтернативно предоставьте общий fetchSessions(limit, offset) API и событие/флаг isLastPage для infinite-scroll или кнопки "Load more".
39-50: Приведениеas anyскрывает несоответствие типов.Объект содержит дополнительные поля (
status,user_prompt,completed_at), которых нет в типеIndexedSession. Использованиеas anyобходит проверку типов, что может привести к ошибкам при рефакторинге.Рекомендуется расширить тип
IndexedSessionили создать отдельный типSDKSessionдля UI, чтобы сохранить типобезопасность.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useSessions.ts` around lines 39 - 50, The mapped session object in useSessions.ts is being cast with "as any", hiding type mismatches between the returned SDK shape and our IndexedSession type; instead, define a proper UI-facing type (e.g., SDKSession or extend IndexedSession) that includes status, user_prompt, completed_at and use that type for the mapped object in sessions.value (or adapt fields to match IndexedSession) so you can remove "as any" and restore type safety for the mapping in the sessions.value = (result.sessions || []).map(...) block.internal/worker/handlers_vault.go (1)
313-317: Отсутствует логирование ошибки и детали в HTTP-ответе.В отличие от других обработчиков в этом файле (например,
handleListCredentials,handleDeleteCredential), при ошибке удаления не логируется сама ошибка и не включаются её детали в ответ. Это затрудняет диагностику проблем.♻️ Предлагаемое исправление
deleted, err := s.observationStore.DeleteOrphanedCredentials(r.Context(), fingerprint) if err != nil { - http.Error(w, "failed to delete orphaned credentials", http.StatusInternalServerError) + log.Error().Err(err).Msg("delete orphaned credentials failed") + http.Error(w, "delete orphaned credentials: "+err.Error(), http.StatusInternalServerError) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_vault.go` around lines 313 - 317, В обработчике где вызывается s.observationStore.DeleteOrphanedCredentials(r.Context(), fingerprint) добавьте логирование ошибки через существующий logger (или s.logger/processLogger, использовать тот же логгер, что и в других обработчиках) и верните более информативный HTTP-ответ: при err вызовите logger.Errorf/Log with context including fingerprint и err, затем используйте http.Error(w, fmt.Sprintf("failed to delete orphaned credentials: %v", err), http.StatusInternalServerError) чтобы включить детали ошибки в ответ; ориентируйтесь на реализацию handleListCredentials и handleDeleteCredential для стиля логирования и формата ответа.
🤖 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.go`:
- Around line 1580-1583: Функция Rollback в записи миграции (Rollback func(tx
*gorm.DB) error) сейчас всегда возвращает nil, давая ложное подтверждение
успешного отката при фактически необратимом удалении таблицы; замените
реализацию Rollback в этом месте файла migrations.go: либо реализуйте реальное
восстановление таблицы через tx (например вызвать tx.Exec/tx.CreateTable с
нужной схемой) если возможна корректная реверсия, либо явно возвращайте ошибку
(fmt.Errorf или errors.New) с понятным сообщением вроде "irreversible migration:
table X cannot be restored" чтобы сигнализировать о невозможности отката.
Укажите имя миграции/таблицы из той же записи при формировании сообщения для
однозначной идентификации.
In `@internal/db/gorm/observation_store.go`:
- Around line 1530-1538: The DeleteOrphanedCredentials method is deleting from
the observations table without limiting to credential-type records; update
DeleteOrphanedCredentials (in ObservationStore) to add a WHERE filter
restricting deletions to only credential records (e.g. add a condition like type
= 'credential' or equivalent column/enum check) so only Observation rows
representing credentials with non-null encrypted_secret and a non-matching
encryption_key_fingerprint are removed; ensure the new Where clause is combined
with the existing conditions before calling Delete(&Observation{}).
In `@internal/db/gorm/session_store.go`:
- Around line 218-221: В запросе q.Order("started_at_epoch DESC") (в коде, где
вызывается Find(&sessions)) сортировка по одному полю может давать нестабильный
порядок при равных timestamp; измените выражение Order, чтобы добавить вторичную
детерминированную сортировку по уникальному ключу записи (например, первичному
ключу "id" или полю с UUID) — например: q.Order("started_at_epoch DESC, id
DESC") — сохранив направление сортировки одинаковым, чтобы обеспечить стабильную
пагинацию при использовании Limit/Offset и Find(&sessions).
---
Nitpick comments:
In `@internal/worker/handlers_sessions.go`:
- Around line 421-429: The handler currently swallows invalid limit/offset query
values and falls back to defaults; update the parsing in the sessions listing
handler so that when r.URL.Query().Get("limit") or "offset" is present but fails
validation (non-integer, limit <=0 or >200, or offset <0) the handler returns a
400 response instead of using defaults. Locate the parsing block around
limit/offset in internal/worker/handlers_sessions.go (the code using
strconv.Atoi and variables limit/offset) and change it to validate
presence+value, and on invalid input call the existing HTTP error response path
(e.g., http.Error or the handler's standard JSON error responder) with a clear
message about the bad parameter and stop further processing.
In `@internal/worker/handlers_vault.go`:
- Around line 313-317: В обработчике где вызывается
s.observationStore.DeleteOrphanedCredentials(r.Context(), fingerprint) добавьте
логирование ошибки через существующий logger (или s.logger/processLogger,
использовать тот же логгер, что и в других обработчиках) и верните более
информативный HTTP-ответ: при err вызовите logger.Errorf/Log with context
including fingerprint и err, затем используйте http.Error(w, fmt.Sprintf("failed
to delete orphaned credentials: %v", err), http.StatusInternalServerError) чтобы
включить детали ошибки в ответ; ориентируйтесь на реализацию
handleListCredentials и handleDeleteCredential для стиля логирования и формата
ответа.
In `@ui/src/composables/useSessions.ts`:
- Around line 20-21: Remove the dead reactive variables filterFrom and filterTo
from useSessions.ts (delete their declarations and remove them from the returned
object) because fetchSDKSessions no longer uses them; if date filtering is
intended later, replace removal with a single TODO comment near fetchSDKSessions
and keep only the TODOed placeholders, but do not return unused refs from the
composable.
- Around line 34-35: В текущем useSessions композиционном файле жёстко
зафиксированы параметры пагинации (limit: 50, offset: 0) — это блокирует доступ
к сессиям за пределами первых 50. Измените реализацию в useSessions так, чтобы
limit был конфигурируемым (по умолчанию 50) и offset — реактивным состоянием, и
добавьте метод (например loadMore или fetchMoreSessions) который увеличивает
offset и вызывает существующую функцию загрузки (например fetchSessions или
getSessions) для подгрузки следующей страницы; альтернативно предоставьте общий
fetchSessions(limit, offset) API и событие/флаг isLastPage для infinite-scroll
или кнопки "Load more".
- Around line 39-50: The mapped session object in useSessions.ts is being cast
with "as any", hiding type mismatches between the returned SDK shape and our
IndexedSession type; instead, define a proper UI-facing type (e.g., SDKSession
or extend IndexedSession) that includes status, user_prompt, completed_at and
use that type for the mapped object in sessions.value (or adapt fields to match
IndexedSession) so you can remove "as any" and restore type safety for the
mapping in the sessions.value = (result.sessions || []).map(...) block.
🪄 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: f15d36d0-b1b9-4946-b757-842a47f5f55f
📒 Files selected for processing (11)
internal/db/gorm/migrations.gointernal/db/gorm/observation_store.gointernal/db/gorm/session_store.gointernal/worker/handlers_sessions.gointernal/worker/handlers_vault.gointernal/worker/service.goui/src/components/layout/AppSidebar.vueui/src/composables/useSessions.tsui/src/types/api.tsui/src/utils/api.tsui/src/views/HomeView.vue
There was a problem hiding this comment.
Code Review
This pull request introduces several significant features and cleanups. It adds a new paginated sessions list view, though the frontend pagination controls are not yet implemented. It also relocates the info sidebar into the main navigation panel, improving the layout of the home view. On the backend, a new endpoint is added to clean up orphaned credentials, and a database migration is included to drop an unused table. The code changes are generally well-structured. I've added a couple of suggestions on the frontend to improve type safety and to point out the missing pagination implementation.
Column updated_at_epoch doesn't exist on observations table. The update was crashing all edit_observation calls with SQLSTATE 42703.
- DeleteOrphanedCredentials now filters by type='credential' (CRIT fix) - Session listing sort includes id DESC for deterministic pagination - edit_observation fixed (removed non-existent updated_at_epoch column)
🤖 PR Review MCP State (auto-managed, do not edit){
"version": 2,
"parentChildren": {},
"resolvedNitpicks": {
"coderabbit-nitpick-9a80535a-421": {
"resolvedAt": "2026-03-23T00:27:22.115Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-e9af09c3-20": {
"resolvedAt": "2026-03-23T00:27:24.788Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-fb25fb6c-34": {
"resolvedAt": "2026-03-23T00:27:27.580Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-ee3ab6ce-39": {
"resolvedAt": "2026-03-23T00:27:30.064Z",
"resolvedBy": "agent"
},
"coderabbit-nitpick-84d484f0-313": {
"resolvedAt": "2026-03-23T00:27:32.700Z",
"resolvedBy": "agent"
}
},
"updatedAt": "2026-03-23T00:27:33.165Z"
} |
The 'english' dictionary drops Cyrillic tokens and mishandles product
names like "SocratiCode". Combining it with 'simple' (no stemming, no
stopword removal) ensures all tokens are indexed verbatim, so mixed-
language queries like "socraticode сломался" correctly find memories.
- Migration 075: drops and recreates search_vector generated column
as to_tsvector('english',...) || to_tsvector('simple',...); recreates
GIN index as idx_observations_search_vector.
- SearchObservationsFTS, SearchObservationsFTSFiltered,
SearchObservationsFTSScored: all @@ predicates and ts_rank calls
now use (websearch_to_tsquery('english', ?) || websearch_to_tsquery('simple', ?));
query param passed twice to match the two placeholders.
The 'english' dictionary drops Cyrillic tokens and mishandles product
names like "SocratiCode". Combining it with 'simple' (no stemming, no
stopword removal) ensures all tokens are indexed verbatim, so mixed-
language queries like "socraticode сломался" correctly find memories.
- Migration 075: drops and recreates search_vector generated column
as to_tsvector('english',...) || to_tsvector('simple',...); recreates
GIN index as idx_observations_search_vector.
- SearchObservationsFTS, SearchObservationsFTSFiltered,
SearchObservationsFTSScored: all @@ predicates and ts_rank calls
now use (websearch_to_tsquery('english', ?) || websearch_to_tsquery('simple', ?));
query param passed twice to match the two placeholders.
The 'english' dictionary drops Cyrillic tokens and mishandles product
names like "SocratiCode". Combining it with 'simple' (no stemming, no
stopword removal) ensures all tokens are indexed verbatim, so mixed-
language queries like "socraticode сломался" correctly find memories.
- Migration 075: drops and recreates search_vector generated column
as to_tsvector('english',...) || to_tsvector('simple',...); recreates
GIN index as idx_observations_search_vector.
- SearchObservationsFTS, SearchObservationsFTSFiltered,
SearchObservationsFTSScored: all @@ predicates and ts_rank calls
now use (websearch_to_tsquery('english', ?) || websearch_to_tsquery('simple', ?));
query param passed twice to match the two placeholders.
Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Removes the per-project adaptive threshold infrastructure: - Migration 097_drop_project_settings (DROP TABLE project_settings; per plan.md C3 rollback returns IRREVERSIBLE error — data was derived from learning signals) - Delete internal/db/gorm/project_settings_store.go (ProjectSettings model + ProjectSettingsStore type + GetThreshold/AdjustThreshold/ UpsertSettings methods) - internal/search/manager.go: remove projectSettingsStore field and SetProjectSettingsStore method; GetProjectThreshold becomes a thin wrapper returning globalDefault (kept as a wrapper so callers in internal/worker don't need to change — remove together in a later US if wanted) - internal/worker/service.go: remove projectSettingsStore field + init + SetProjectSettingsStore call (3 lines total) - internal/worker/handlers_scoring.go: remove the adaptive threshold adjustment block in the utility-signal handler (it was the only caller of ProjectSettingsStore.AdjustThreshold) - internal/worker/reaper/reaper.go: drop the project_settings entry from the ''tables related to projects.id'' comment Verified: go build ./... — clean go vet ./... — clean go test -short -count=1 ./... — green Per plan.md C7: CodeRabbit-only review. Per plan.md C8: single coherent PR, no atomic-commit split. Plan.md §Phase 4. Closes task #42.
Removes the per-project adaptive threshold infrastructure: - Migration 097_drop_project_settings (DROP TABLE project_settings; per plan.md C3 rollback returns IRREVERSIBLE error — data was derived from learning signals) - Delete internal/db/gorm/project_settings_store.go (ProjectSettings model + ProjectSettingsStore type + GetThreshold/AdjustThreshold/ UpsertSettings methods) - internal/search/manager.go: remove projectSettingsStore field and SetProjectSettingsStore method; GetProjectThreshold becomes a thin wrapper returning globalDefault (kept as a wrapper so callers in internal/worker don't need to change — remove together in a later US if wanted) - internal/worker/service.go: remove projectSettingsStore field + init + SetProjectSettingsStore call (3 lines total) - internal/worker/handlers_scoring.go: remove the adaptive threshold adjustment block in the utility-signal handler (it was the only caller of ProjectSettingsStore.AdjustThreshold) - internal/worker/reaper/reaper.go: drop the project_settings entry from the ''tables related to projects.id'' comment Verified: go build ./... — clean go vet ./... — clean go test -short -count=1 ./... — green Per plan.md C7: CodeRabbit-only review. Per plan.md C8: single coherent PR, no atomic-commit split. Plan.md §Phase 4. Closes task #42. Co-authored-by: Kirill Turanskiy <thebtf@users.noreply.github.com>
Summary
GET /api/sessions/listqueriessdk_sessionstable with pagination + project filter. Frontend switched fromfetchIndexedSessionstofetchSDKSessions. Transcript search preserved.Sidebar.vueremoved from HomeView — timeline gets full width.DROP TABLE IF EXISTS memory_blocks(unused since creation by migration 024)DELETE /api/vault/orphaned-credentialsendpoint purges credentials encrypted with a lost key (fingerprint mismatch)Test plan
go build ./...passescd ui && npm run buildpassesSummary by CodeRabbit
Новые функции
Улучшения