feat: observation browser, detail view, semantic search (Phases 4-5)#20
Merged
Conversation
Create api_tokens table for client token authentication with bcrypt hash storage, prefix-based lookup index, and usage tracking columns.
GORM-based store with Create, List, FindByPrefix, Revoke, IncrementStats, IncrementErrorCount, GetByID, and BatchIncrementStats methods. APIToken model added to models.go.
Handlers for POST /api/auth/login (master token validation + HMAC cookie), POST /api/auth/logout (cookie clear), GET /api/auth/me (status check), GET/POST /api/auth/tokens (list/create), DELETE /api/auth/tokens/:id (revoke). All handlers include swaggo annotations.
TokenAuth now supports three auth methods: master token (admin), client API tokens (eng_* prefix with bcrypt verification and scope enforcement), and HMAC-SHA256 signed session cookies (dashboard). Read-only tokens are restricted to GET + whitelisted POST endpoints. HMAC cookie key derived deterministically from master token via SHA-256.
Add public routes (login, logout) and authenticated routes (me, tokens CRUD) to setupRoutes(). Create TokenStore in initializeAsync() and wire it into TokenAuth middleware after DB initialization.
Background goroutine reads token IDs from a buffered channel and flushes accumulated request counts to the database every 5 seconds, reducing per-request UPDATE overhead for client token authentication.
New handlers in handlers_vault.go:
- GET /api/vault/credentials — list credentials (no values)
- GET /api/vault/credentials/{name} — get with decrypted value
- POST /api/vault/credentials — store credential
- DELETE /api/vault/credentials/{name} — delete credential
- GET /api/vault/status — vault encryption status
All handlers delegate to existing crypto.Vault and ObservationStore.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_tags.go:
- POST /api/observations/{id}/tags — add/remove/set tags
- GET /api/observations/by-tag/{tag} — list observations by tag
Logic mirrors MCP tag_observation and get_observations_by_tag handlers.
Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_sessions_rest.go: - GET /api/sessions-index — list indexed sessions - GET /api/sessions-index/search — full-text search transcripts Uses /api/sessions-index path to avoid conflict with existing /api/sessions live session management routes. Includes swaggo annotations for OpenAPI docs.
New handlers in handlers_maintenance.go: - POST /api/maintenance/consolidation — trigger consolidation cycle - POST /api/maintenance/run — trigger full maintenance - GET /api/maintenance/stats — maintenance statistics Delegates to existing consolidationScheduler and maintenanceService. Includes swaggo annotations for OpenAPI docs.
New handler in handlers_analytics.go: - GET /api/analytics/trends — temporal trends (obs per period) Supports daily/weekly/hourly grouping with project filter. Logic mirrors MCP get_temporal_trends handler. Includes swaggo annotations for OpenAPI docs.
Wire all new REST handlers into the requireReady + auth route group:
- /api/vault/* (5 endpoints)
- /api/observations/{id}/tags, /api/observations/by-tag/{tag}
- /api/sessions-index, /api/sessions-index/search
- /api/maintenance/* (3 endpoints)
- /api/analytics/trends
Add composables for observation CRUD (useObservation), generic pagination (usePagination), and search state (useSearch). Add API functions for observation detail, update, archive, feedback, and search endpoints. Add SearchResult types and rejected[] field to Observation type.
Replace placeholder with paginated observation list using server-side pagination (20 items/page). Add project dropdown, type filter pills, and concept filter. Each row is clickable to navigate to detail view.
Replace placeholder with full observation detail view including: - Read-only display of all fields (narrative, facts, concepts, files, metadata) - Rejected alternatives section for decision observations - Inline edit mode with ObservationEditor component (explicit save/discard) - beforeunload warning for unsaved changes - Archive with confirmation dialog - Feedback buttons (thumbs up/down)
Add SearchBar with "/" hotkey focus and compact/expanded modes. Add SearchResults displaying relevance percentage, type badges, concept tags, and rejected alternatives for decisions. Implement SearchView with project filter, decision mode toggle, and URL query parameter support for header search integration.
Replace inline search form with reusable SearchBar component in compact mode. Adds "/" hotkey focus and consistent styling across header and dedicated search page.
ОбзорДобавляет комплексную систему управления API-токенами с поддержкой аутентификации клиентов, включая миграцию БД, CRUD-операции токенов, аутентификацию по префиксу токена, отслеживание статистики использования и полнофункциональный веб-интерфейс на Vue 3 с маршрутизацией и компонентами. Изменения
Диаграммы последовательностиsequenceDiagram
participant Client
participant API as Worker API
participant Auth as TokenAuth
participant Store as TokenStore
participant DB as Database
Client->>API: POST /api/auth/tokens (create)
API->>Auth: Request with Master Token
Auth->>Auth: Verify master token
Auth->>API: Admin access granted
API->>API: Generate token (eng_ + 32 hex)
API->>API: Compute bcrypt hash
API->>Store: Create(name, tokenHash, prefix, scope)
Store->>DB: INSERT api_tokens
DB-->>Store: Token ID
Store-->>API: APIToken
API-->>Client: { id, name, raw_token, scope }
Note over Client,DB: Later: Client API Request
Client->>API: GET /api/observations (with X-Auth-Token: eng_...)
API->>Auth: Authenticate request
Auth->>Auth: Extract prefix from token
Auth->>Store: FindByPrefix(prefix)
Store->>DB: SELECT WHERE token_prefix=? AND NOT revoked
DB-->>Store: APIToken
Store-->>Auth: Token found
Auth->>Auth: Verify bcrypt hash
Auth->>Auth: Check if revoked
Auth->>Auth: Enforce read-only scope
Auth->>Auth: Signal stats update async
Auth-->>API: Client access with scoped permissions
API-->>Client: { observations: [...] }
Store->>Store: Batch flush every 5s
Store->>DB: UPDATE api_tokens SET request_count+=...
DB-->>Store: Updated
sequenceDiagram
participant Browser
participant UI as Vue App
participant Router
participant API as Worker API
participant Auth as useAuth
Browser->>UI: Load App
UI->>UI: App.vue mounted
UI->>Auth: checkAuth()
Auth->>API: GET /api/auth/me
API-->>Auth: { authenticated: true, role: "admin" }
Auth-->>UI: authenticated=true, loading=false
UI->>Router: Render AppSidebar + AppHeader + router-view
UI->>Browser: Show authenticated layout
Browser->>Browser: Click "Observations" nav item
UI->>Router: beforeEach guard
Router->>Auth: Check if route public
Auth-->>Router: Not public, authenticated✓
Router-->>UI: Navigation allowed
Router->>UI: Render ObservationsView
UI->>UI: fetchPage() via usePagination
UI->>API: GET /api/observations?limit=20&offset=0&project=...
API-->>UI: { items: [...], total: 150 }
UI->>Browser: Render paginated list
Browser->>Browser: Click observation
UI->>Router: Navigate to observation-detail
Router->>UI: Render ObservationDetailView
UI->>API: GET /api/observations/{id}
API-->>UI: Observation data
UI->>Browser: Show detail with edit/archive options
Оценка сложности кода🎯 4 (Complex) | ⏱️ ~60 минут Возможно связанные PR
Рекомендуемые метки
Стихотворение
🚥 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)
📝 Coding Plan
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 introduces a major expansion of the dashboard's capabilities, transforming it into a more robust and interactive application. The core focus is on enhancing the user's ability to browse, manage, and search observations, alongside foundational improvements to authentication and backend services. The changes lay the groundwork for future phases by providing a structured UI, comprehensive data interaction patterns, and secure API access. 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
|
![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 introduces a significant set of features for the dashboard, including observation browsing, a detail view, and semantic search capabilities. The changes are extensive, touching both the backend and frontend. On the backend, a new api_tokens table and corresponding authentication logic have been added, providing a robust system for client and session-based auth. New handlers for analytics, maintenance, tags, and the vault have also been introduced. The frontend has been substantially refactored to use vue-router, creating a more structured and scalable single-page application. My review focuses on a few performance and maintainability improvements in the new backend handlers. The overall implementation is solid and a great step forward for the project.
Comment on lines
+79
to
+105
| for _, o := range obs { | ||
| if o.CreatedAtEpoch < startEpoch { | ||
| continue | ||
| } | ||
| totalInRange++ | ||
|
|
||
| created := time.UnixMilli(o.CreatedAtEpoch) | ||
| var key string | ||
| switch groupBy { | ||
| case "week": | ||
| year, week := created.ISOWeek() | ||
| key = fmt.Sprintf("%d-W%02d", year, week) | ||
| case "hour_of_day": | ||
| key = fmt.Sprintf("%02d:00", created.Hour()) | ||
| default: // day | ||
| key = created.Format("2006-01-02") | ||
| } | ||
| buckets[key]++ | ||
|
|
||
| // Track type distribution | ||
| typeDistribution[string(o.Type)]++ | ||
|
|
||
| // Track top concepts | ||
| for _, c := range o.Concepts { | ||
| conceptCounts[c]++ | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation fetches all recent observations into memory and then performs grouping and aggregation in Go. This can be inefficient and consume a lot of memory if the number of observations is large. For better performance and scalability, consider performing these aggregations directly in the database using SQL GROUP BY queries. For example, you could use DATE_TRUNC for daily/weekly bucketing and jsonb_array_elements_text (if using JSONB) or similar functions to count concepts.
Comment on lines
+126
to
+132
| for i := 0; i < len(topConcepts) && i < 10; i++ { | ||
| for j := i + 1; j < len(topConcepts); j++ { | ||
| if topConcepts[j].count > topConcepts[i].count { | ||
| topConcepts[i], topConcepts[j] = topConcepts[j], topConcepts[i] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The sorting algorithm used here is a bubble sort, which has a time complexity of O(n^2). While it works for small slices, it's inefficient for larger ones. It's more idiomatic and performant to use Go's standard library sort.Slice function, which provides an optimized sorting implementation (typically IntroSort with O(n log n) average complexity).
sort.Slice(topConcepts, func(i, j int) bool {
return topConcepts[i].count > topConcepts[j].count
})
Comment on lines
+360
to
+369
| for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} { | ||
| if len(msg) >= len(s) { | ||
| for i := 0; i <= len(msg)-len(s); i++ { | ||
| if msg[i:i+len(s)] == s { | ||
| return true | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return false |
There was a problem hiding this comment.
This function reimplements the functionality of strings.Contains in a less efficient way. The nested loops are not necessary. You can simplify this by iterating through the search strings and using the standard library function.
for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} {
if strings.Contains(msg, s) {
return true
}
}
return false
Comment on lines
+153
to
+155
| if len(snippet) > 200 { | ||
| snippet = snippet[:200] | ||
| } |
There was a problem hiding this comment.
Truncating a string by slicing bytes (snippet[:200]) can lead to invalid UTF-8 if a multi-byte character is cut in the middle. This can cause issues with JSON marshalling or rendering. It's safer to truncate based on the number of runes. You'll need to import unicode/utf8 for this.
Suggested change
| if len(snippet) > 200 { | |
| snippet = snippet[:200] | |
| } | |
| if utf8.RuneCountInString(snippet) > 200 { | |
| snippet = string([]rune(snippet)[:200]) | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (15)
ui/src/components/layout/ConfirmDialog.vue (1)
18-50: Добавьте базовую доступность для модального окна (dialog semantics + Esc).Сейчас модалка рабочая, но без
role="dialog"/aria-modalи обработки Esc ухудшается UX для клавиатурной и assistive-навигации.♿ Вариант точечного улучшения
- <div v-if="show" class="fixed inset-0 z-50 flex items-center justify-center p-4"> + <div + v-if="show" + class="fixed inset-0 z-50 flex items-center justify-center p-4" + `@keydown.esc`="emit('cancel')" + > <!-- Backdrop --> <div class="absolute inset-0 bg-black/60 backdrop-blur-sm" `@click`="emit('cancel')" /> <!-- Dialog --> - <div class="relative glass border border-white/10 rounded-xl p-6 max-w-sm w-full shadow-2xl"> - <h3 class="text-lg font-semibold text-white mb-2">{{ title }}</h3> - <p class="text-sm text-slate-400 mb-6">{{ message }}</p> + <div + class="relative glass border border-white/10 rounded-xl p-6 max-w-sm w-full shadow-2xl" + role="dialog" + aria-modal="true" + aria-labelledby="confirm-dialog-title" + aria-describedby="confirm-dialog-message" + tabindex="-1" + `@click.stop` + > + <h3 id="confirm-dialog-title" class="text-lg font-semibold text-white mb-2">{{ title }}</h3> + <p id="confirm-dialog-message" class="text-sm text-slate-400 mb-6">{{ message }}</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/ConfirmDialog.vue` around lines 18 - 50, The modal lacks basic accessibility: add dialog semantics and Esc handling. On the dialog container element (the div with classes "relative glass ...", inside ConfirmDialog.vue) add role="dialog", aria-modal="true", aria-labelledby pointing to the title ID and aria-describedby pointing to the message ID; give the <h3> (title) and the <p> (message) stable IDs (e.g., titleId/messageId). Also add an Esc key handler (e.g., `@keydown.escape.window` or a keydown listener that emits('cancel')) so pressing Esc closes the dialog, and ensure the dialog is focusable (tabindex="-1") and focused when opened (e.g., focus it when show becomes true) to support keyboard/assistive navigation.internal/worker/handlers_analytics.go (2)
126-135: Рекомендуется использоватьsort.Sliceвместо ручной сортировки.Текущая реализация — частичная сортировка выбором с O(n²) сложностью. Стандартная библиотека предоставляет более эффективный и читаемый вариант.
♻️ Предлагаемое исправление
+ "sort"- for i := 0; i < len(topConcepts) && i < 10; i++ { - for j := i + 1; j < len(topConcepts); j++ { - if topConcepts[j].count > topConcepts[i].count { - topConcepts[i], topConcepts[j] = topConcepts[j], topConcepts[i] - } - } - } - if len(topConcepts) > 10 { - topConcepts = topConcepts[:10] - } + sort.Slice(topConcepts, func(i, j int) bool { + return topConcepts[i].count > topConcepts[j].count + }) + if len(topConcepts) > 10 { + topConcepts = topConcepts[:10] + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_analytics.go` around lines 126 - 135, Replace the manual O(n²) selection-sort loop over topConcepts with sort.Slice: import "sort", call sort.Slice(topConcepts, func(i, j int) bool { return topConcepts[i].count > topConcepts[j].count }), then truncate to topConcepts = topConcepts[:min(10, len(topConcepts))] (or keep the existing slice-length guard) so the slice is sorted descending by count and then limited to 10; update any references to topConcepts in handlers_analytics.go accordingly.
53-58: Молчаливый fallback при невалидных значенияхdaysможет скрыть ошибки пользователя.Если передано невалидное значение (например,
days=-5илиdays=abc), код молча использует дефолтное значение 30 без уведомления клиента. Рассмотрите возможность возврата ошибки 400 для явно невалидных значений.♻️ Предлагаемое исправление
days := 30 if val := r.URL.Query().Get("days"); val != "" { - if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 && parsed <= 365 { + parsed, err := strconv.Atoi(val) + if err != nil { + http.Error(w, "invalid days parameter: must be an integer", http.StatusBadRequest) + return + } + if parsed < 1 || parsed > 365 { + http.Error(w, "days must be between 1 and 365", http.StatusBadRequest) + return + } days = parsed - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_analytics.go` around lines 53 - 58, When the "days" query param is present but invalid, don't silently fall back to 30; validate r.URL.Query().Get("days") using strconv.Atoi and if parsing fails or the parsed value is <=0 or >365, write a 400 Bad Request response (e.g., via http.Error or similar) describing the invalid "days" value instead of assigning the default to the days variable; otherwise set days to the parsed value. Ensure this change is applied where the current logic around days, r.URL.Query().Get("days") and strconv.Atoi is implemented so clients receive explicit feedback for bad input.ui/src/composables/useObservation.ts (1)
39-64: ОтсутствуетAbortSignalдля операцииsave.Функции
save,archiveиfeedbackне используютAbortController, в отличие отload. Если компонент размонтируется во время выполнения этих операций, запросы продолжат выполняться, а обновление состояния может вызвать предупреждения Vue или утечки.♻️ Предлагаемое исправление для save
async function save(updates: { title?: string subtitle?: string narrative?: string scope?: string facts?: string[] concepts?: string[] }) { const id = observation.value?.id if (!id) return + abortController?.abort() + abortController = new AbortController() + saving.value = true error.value = null try { - const result = await updateObservation(id, updates) + const result = await updateObservation(id, updates, abortController.signal) observation.value = result.observation return result.observation } catch (err) { + if (err instanceof Error && err.name === 'AbortError') return error.value = err instanceof Error ? err.message : 'Failed to save observation' console.error('[useObservation] Save error:', err) throw err } finally { saving.value = false } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/composables/useObservation.ts` around lines 39 - 64, The save/archive/feedback functions lack AbortSignal handling and should mirror the load pattern: create an AbortController at the start of save (and similarly in archive and feedback), pass controller.signal into the API call (e.g., updateObservation(id, updates, { signal }) or updateObservation({ id, updates, signal }) matching the API signature), register onUnmounted(() => controller.abort()) (or reuse a per-operation controller and abort previous controller before creating a new one) so requests are cancelled when the component unmounts, and handle abort errors in the existing catch block without setting a user-facing error; reference the save function and its call to updateObservation as well as the existing load implementation for the exact abort/cleanup pattern.internal/db/gorm/migrations.go (1)
1217-1229: Рассмотрите добавление уникального ограничения наtoken_hash.Для предотвращения коллизий хэшей (маловероятно, но возможно при ошибках в коде генерации) и обеспечения целостности данных рекомендуется добавить
UNIQUEconstraint наtoken_hash.♻️ Предлагаемое изменение
`CREATE TABLE IF NOT EXISTS api_tokens ( id UUID PRIMARY KEY DEFAULT gen_random_uuid(), name TEXT NOT NULL UNIQUE, - token_hash TEXT NOT NULL, + token_hash TEXT NOT NULL UNIQUE, token_prefix TEXT NOT NULL,🤖 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 1217 - 1229, Add a UNIQUE constraint on the api_tokens.token_hash to prevent hash collisions: update the CREATE TABLE statement that defines api_tokens to mark token_hash as UNIQUE (or add a UNIQUE constraint name) and also add a migration step to enforce uniqueness on existing databases by creating a unique index or ALTER TABLE ... ADD CONSTRAINT (use IF NOT EXISTS semantics where supported). Locate the SQL string in migrations.go that builds the api_tokens table (the CREATE TABLE IF NOT EXISTS api_tokens ... block) and modify it, and add an accompanying migration function/step in the same migrations list to create the unique index/constraint for existing data.internal/worker/handlers_tags.go (1)
53-63: Рассмотрите возможность разрешить пустыеtagsдля действия "set".Текущая логика запрещает пустой массив
tags, что делает невозможным удаление всех тегов через действие "set". Если это ограничение намеренное, рекомендуется добавить комментарий; если нет — разрешить пустой массив для "set".♻️ Предлагаемое исправление
- if len(req.Tags) == 0 { - http.Error(w, "tags is required", http.StatusBadRequest) - return - } + if len(req.Tags) == 0 && req.Action != "set" { + http.Error(w, "tags is required for add/remove actions", http.StatusBadRequest) + return + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_tags.go` around lines 53 - 63, The current check rejects an empty req.Tags for all actions, preventing clearing all tags via the "set" action; update the validation in the handler that currently checks len(req.Tags) == 0 so that it only returns a 400 when tags are empty for actions other than "set" (i.e., allow an empty slice when req.Action == "set"), or if the empty-for-set behavior was intentional, add a clear inline comment next to that validation mentioning the decision; reference req.Tags and req.Action in your change.ui/src/components/layout/Pagination.vue (1)
69-75: Рекомендуется добавить aria-label для улучшения доступности.Кнопки навигации "Previous" и "Next" используют только иконки без текстового содержимого. Для пользователей скринридеров рекомендуется добавить
aria-label.♻️ Предлагаемое исправление
<button class="px-2 py-1 rounded text-slate-400 hover:text-white hover:bg-slate-800/50 transition-colors disabled:opacity-30 disabled:cursor-not-allowed" :disabled="currentPage <= 1" `@click`="goToPage(currentPage - 1)" + aria-label="Previous page" > <i class="fas fa-chevron-left text-xs" /> </button>И аналогично для кнопки "Next":
<button class="px-2 py-1 rounded text-slate-400 hover:text-white hover:bg-slate-800/50 transition-colors disabled:opacity-30 disabled:cursor-not-allowed" :disabled="currentPage >= totalPages" `@click`="goToPage(currentPage + 1)" + aria-label="Next page" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/Pagination.vue` around lines 69 - 75, The previous/next navigation buttons in Pagination.vue (the <button> elements that call goToPage and check currentPage) lack aria-labels for screen readers; update both button elements (the one with `@click`="goToPage(currentPage - 1)" and the corresponding next button with `@click`="goToPage(currentPage + 1)") to include descriptive aria-label attributes like "Previous page" and "Next page" respectively so assistive technologies can announce their purpose.internal/worker/token_stats.go (1)
70-72: Рассмотрите передачу контекста вflushTokenStats.Использование
context.Background()означает, что операция сброса статистики не будет учитывать таймауты или отмену при завершении работы сервиса. Для консистентности и возможности отмены рекомендуется передавать контекст как параметр.♻️ Предлагаемое исправление
-func (s *Service) flushTokenStats(counts map[string]int) { +func (s *Service) flushTokenStats(ctx context.Context, counts map[string]int) { if len(counts) == 0 { return } @@ -67,7 +67,7 @@ func (s *Service) flushTokenStats(counts map[string]int) { return } - if err := store.BatchIncrementStats(context.Background(), counts); err != nil { + if err := store.BatchIncrementStats(ctx, counts); err != nil { log.Warn().Err(err).Int("tokens", len(counts)).Msg("auth: failed to flush token stats") } }И соответственно обновите вызовы:
case <-ctx.Done(): // Final flush on shutdown - s.flushTokenStats(pending) + s.flushTokenStats(context.Background(), pending) // Background for final flush return ... case <-ticker.C: - s.flushTokenStats(pending) + s.flushTokenStats(ctx, pending)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/token_stats.go` around lines 70 - 72, The flushTokenStats function currently uses context.Background() which ignores cancellation/timeouts; change flushTokenStats to accept a context.Context parameter and pass that ctx into store.BatchIncrementStats (replace context.Background() with ctx), then update all callers of flushTokenStats to forward their available context (e.g., request or service ctx) so the batch flush respects cancellation and deadlines; reference functions: flushTokenStats and store.BatchIncrementStats.ui/src/router/index.ts (1)
4-71: Рекомендуется добавить catch-all маршрут для обработки 404.Отсутствует маршрут для обработки несуществующих путей. Пользователи, переходящие по неверному URL, не получат обратной связи.
♻️ Предлагаемое исправление
{ path: '/tokens', name: 'tokens', component: () => import('@/views/TokensView.vue'), }, + { + path: '/:pathMatch(.*)*', + name: 'not-found', + component: () => import('@/views/NotFoundView.vue'), + }, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/router/index.ts` around lines 4 - 71, The routes array lacks a catch-all 404 route; add a final route entry to routes (after existing entries) with a path like '/:pathMatch(.*)*', name 'not-found' and a component that renders your NotFound view (e.g., lazy-import '@/views/NotFoundView.vue') or redirect to an existing route; ensure the route is appended at the end so Vue Router will use it for unmatched URLs.ui/src/views/LoginView.vue (1)
29-30: Рассмотрите логирование ошибки соединения для отладки.В блоке
catchошибка игнорируется, что затрудняет диагностику проблем с подключением. Рекомендуется логировать ошибку в консоль для упрощения отладки.♻️ Предлагаемое исправление
- } catch { + } catch (err) { + console.error('Login failed:', err) error.value = 'Connection error. Is the server running?'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/LoginView.vue` around lines 29 - 30, The catch block currently swallows the exception and only sets error.value; update the catch to accept the error (e) and log it (e.g., console.error) for debugging while keeping the user-facing assignment to error.value; locate the catch associated with the login flow in LoginView.vue where error.value = 'Connection error. Is the server running?' and change it to catch (e) { console.error('Login connection error', e); error.value = 'Connection error. Is the server running?'; }.ui/src/components/layout/AppSidebar.vue (1)
76-90: В свернутом режиме добавьте явные доступные имена.Когда
collapsed === true, у ссылок и кнопок остаются только иконки.titleздесь слабая замена доступному имени, поэтому лучше явно добавитьaria-labelилиsr-onlyтекст.💡 Пример
<router-link @@ :title="collapsed ? item.label : undefined" + :aria-label="collapsed ? item.label : undefined" > @@ <button @@ title="Logout" + :aria-label="collapsed ? 'Logout' : undefined" `@click`="handleLogout" > @@ <button @@ :title="collapsed ? 'Expand sidebar' : 'Collapse sidebar'" + :aria-label="collapsed ? (collapsed ? 'Expand sidebar' : 'Collapse sidebar') : undefined" `@click`="toggleCollapse" >Also applies to: 96-112, 115-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layout/AppSidebar.vue` around lines 76 - 90, The links rendered by the router-link v-for (using navItems, isActive, collapsed, item.label and item.icon) lack accessible names when collapsed; update the router-link and other icon-only buttons (the blocks referenced at lines 96-112 and 115-143) to provide an explicit accessible name by adding aria-label="{item.label}" when collapsed is true (or include an sr-only <span> with item.label that is conditionally rendered only when collapsed) so screen readers receive a clear name while preserving the visual collapsed icon-only UI.internal/worker/handlers_auth.go (1)
358-370: Используйтеstrings.Containsвместо ручного поиска.Функция
containsDuplicateKeyреализует ручной поиск подстроки, который менее читаем и потенциально менее оптимизирован, чем стандартная библиотека.♻️ Предлагаемое исправление
+import "strings" + // containsDuplicateKey checks error message for duplicate key indicators. func containsDuplicateKey(msg string) bool { - for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} { - if len(msg) >= len(s) { - for i := 0; i <= len(msg)-len(s); i++ { - if msg[i:i+len(s)] == s { - return true - } - } - } + for _, s := range []string{"duplicate key", "23505", "UNIQUE constraint"} { + if strings.Contains(msg, s) { + return true + } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/handlers_auth.go` around lines 358 - 370, The containsDuplicateKey function currently does manual substring scanning; replace the manual loops with calls to strings.Contains for each pattern by iterating over the slice {"duplicate key","23505","UNIQUE constraint"} and returning true if any strings.Contains(msg, s) is true; also add the "strings" import (or include it in the existing import block) and remove the manual index-based logic to simplify and rely on the stdlib implementation.ui/src/views/ObservationDetailView.vue (2)
269-305: Рассмотрите кэширование результатаsplitPath.Функция
splitPathвызывается дважды для каждого файла в шаблоне (дляrootиpath). Хотя это незначительная оптимизация, можно использовать вычисляемое свойство или v-memo для больших списков файлов.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/ObservationDetailView.vue` around lines 269 - 305, The template calls splitPath(file) twice per file; cache its result by creating computed properties (e.g., filesReadSplit and filesModifiedSplit) that map observation.files_read and observation.files_modified to arrays of objects { orig: file, root: ..., path: ... } using splitPath once, then update the v-for loops (currently iterating observation.files_read and observation.files_modified) to iterate the new computed arrays and reference file.root/file.path (and use file.orig for :key and :title) so splitPath is called only once per file; alternatively, use Vue 3 v-memo in the same v-for if you prefer to keep logic in-template.
2-2: Удалите неиспользуемый импортonUnmounted.Импортирован
onUnmounted, но в коде используется толькоonBeforeUnmount. Это мёртвый код.🧹 Предлагаемое исправление
-import { ref, computed, onMounted, onUnmounted, onBeforeUnmount } from 'vue' +import { ref, computed, onMounted, onBeforeUnmount } from 'vue'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/views/ObservationDetailView.vue` at line 2, Remove the unused onUnmounted import from the top-level import statement in ObservationDetailView.vue; the file only uses onBeforeUnmount (and other imports like ref, computed, onMounted), so update the import list to exclude onUnmounted to eliminate dead code and lint warnings.internal/worker/service.go (1)
1582-1584: Архитектурное замечание: эндпоинты аутентификации должны оставаться независимыми от состояния БД.Текущая реализация
handleAuthLoginиhandleAuthLogoutкорректна — они не обращаются к БД и используют толькоs.tokenAuth, инициализируемый при запуске. Однако эти эндпоинты намеренно расположены вне группыrequireReady, что означает, что они доступны до полной готовности системы.Если в будущем потребуется добавить логирование попыток входа или аудит в БД, это может нарушить архитектурный принцип, что эти эндпоинты должны быть доступны независимо от состояния БД. Рекомендуется либо задокументировать это требование, либо убедиться, что любое логирование/аудит аутентификации используют асинхронные механизмы без блокирующего ожидания готовности БД.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/worker/service.go` around lines 1582 - 1584, Auth endpoints / handlers handleAuthLogin and handleAuthLogout are intentionally exposed outside the requireReady group and must remain independent of DB readiness (they only use s.tokenAuth); update the codebase to enforce this: add a clear code comment near the router registrations and the handlers (handleAuthLogin, handleAuthLogout) documenting the invariant that these handlers must not perform blocking DB calls, and if you add logging/audit in the future, perform it asynchronously (e.g., fire-and-forget goroutine or enqueue to an async logger/queue) so it never waits for requireReady or DB initialization; alternatively, if synchronous DB audit is required, move those endpoints into the requireReady group—choose one approach and document it next to s.tokenAuth and the router registrations.
🤖 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/token_store.go`:
- Around line 63-77: In TokenStore.Revoke, the code checks result.RowsAffected
before result.Error which can hide real DB errors; change the order in the
Revoke method (function TokenStore.Revoke) to first inspect result.Error and
return it if non-nil, then check result.RowsAffected and return
gorm.ErrRecordNotFound when RowsAffected == 0, leaving the final return as nil
(or result.Error if kept) so that real database errors from the Updates call on
APIToken are not masked.
In `@internal/worker/handlers_auth.go`:
- Around line 118-125: The cookie deletion in the logout flow uses
http.SetCookie with Name sessionCookieName but omits the Secure flag; update the
http.SetCookie call (the cookie literal used to clear sessionCookieName) to
include Secure: true so the cleared cookie matches the login/logout cookie
attributes (SameSite, HttpOnly, Path, MaxAge) for consistency and security.
- Around line 95-102: The cookie is set without the Secure flag; update the
http.SetCookie call that creates the cookie (referencing sessionCookieName,
cookieValue, sessionMaxAge, and SameSite: http.SameSiteStrictMode) to set
Secure: true when running in production (e.g. check an existing isProd/env var
or add one derived from an env var) so cookies are only sent over HTTPS in
production; ensure the boolean is conditional so local/dev HTTP remains
unaffected.
In `@internal/worker/handlers_maintenance.go`:
- Around line 36-41: The handler currently skips JSON parsing when
r.ContentLength <= 0, which breaks chunked requests; remove the ContentLength
check and always attempt decoding from r.Body (if r.Body != nil), using
json.NewDecoder(r.Body).Decode(&req) and treat io.EOF as non-error (i.e., only
return HTTP 400 when err != nil && err != io.EOF) so chunked transfer-encoded
bodies are parsed correctly.
In `@internal/worker/handlers_sessions_rest.go`:
- Around line 32-37: The limit parsing currently trusts any positive
client-provided value; add an explicit upper bound (e.g. define a maxLimit
constant) and clamp the parsed value to that maximum when assigning to limit in
both places where the block appears (the limit variable parsing in
handlers_sessions_rest.go and the similar block at the other endpoint around
lines 117-122). Update the logic that parses strconv.Atoi to: validate parsed >
0, then set limit = min(parsed, maxLimit) (or set limit = maxLimit if parsed >
maxLimit) so the API cannot accept an unbounded large limit.
- Around line 54-56: The handler currently logs the detailed error
(log.Error().Err(err).Msg(...)) but then returns the internal error string to
clients via http.Error(w, "list sessions: "+err.Error(),
http.StatusInternalServerError); change the http.Error call(s) to return a
generic message (e.g., "internal server error" or "list sessions failed")
without including err.Error(), keep the detailed log.Error().Err(err).Msg(...)
unchanged, and apply the same change to the other occurrence around the
http.Error at the later block (the second http.Error call at lines 126-128).
In `@internal/worker/handlers_tags.go`:
- Around line 132-133: В обработчике поиска тегов добавьте парсинг параметра
offset из query и передачу его в объект SearchParams: извлеките offset (default
0) из запроса в функции-обработчике (например в SearchTagsHandler /
соответствующей функции), конвертируйте в int с обработкой ошибок и присвойте в
поле Offset у SearchParams (добавьте поле Offset int в структуру SearchParams,
если его нет), затем используйте это значение при формировании запроса к
репозиторию/DAO так, чтобы Swagger-параметр offset действительно влиял на
пагинацию.
In `@internal/worker/handlers_vault.go`:
- Around line 93-95: Имя credential сейчас допускает символ '/', что ломает
маршруты вида "/{name}"; в функциях handleStoreCredential, handleGetCredential и
handleDeleteCredential (там где читается имя через chi.URLParam(r, "name"))
добавьте валидацию: помимо ненулевой длины запрещайте символ '/' (или
альтернативно требуйте явного url-encoding) и при нарушении возвращайте
http.StatusBadRequest с понятным сообщением; обновите все места, где
используется chi.URLParam(r, "name") (включая аналогичные участки в тех же
обработчиках), чтобы применять одну и ту же проверку/утилиту для
согласованности.
In `@internal/worker/token_stats.go`:
- Around line 42-43: Проблема: чтение из канала ch в выражении case tokenID :=
<-ch не проверяет, закрыт ли канал, поэтому при закрытии будет постоянно
получаться нулевое значение и расти pending[""]. Исправление: в том
select-branch где используется case tokenID := <-ch заменить на tokenID, ok :=
<-ch и при ok==false прекратить обработку (например, выйти из цикла/горутины или
закрыть рабочий цикл), иначе инкрементировать pending[tokenID]; обновите
соответствующий код в месте обработки канала ch и карты pending, чтобы корректно
завершать цикл при закрытии канала.
In `@ui/src/components/layout/AppHeader.vue`:
- Around line 22-31: The restartWorker flow treats failures as success because
fetch('/api/update/restart') isn't checked and waitForWorker() may quietly time
out; update restartWorker to validate the fetch response (check response.ok and
handle non-2xx by throwing or setting isRestarting.value = false and logging)
and modify waitForWorker to reject/throw on timeout or failed readiness so
restartWorker's catch is triggered instead of calling
globalThis.location.reload(); reference the restartWorker function and the
waitForWorker function when making these changes.
In `@ui/src/components/layout/AppSidebar.vue`:
- Around line 42-47: The isActive(item: NavItem) function incorrectly uses
route.path.startsWith(item.path) causing subpath collisions (e.g., /logs-archive
matching /logs); update isActive to return true only for exact match for '/' and
otherwise true when route.path === item.path OR when
route.path.startsWith(item.path + '/') so that segment boundaries are respected;
reference the isActive function and route.path/NavItem to locate and change the
condition accordingly.
In `@ui/src/components/observation/ObservationEditor.vue`:
- Around line 42-57: The computed hasChanges and handleSave currently call
concepts.value.sort(), which mutates the reactive concepts array; change both
comparisons to sort a shallow copy instead (e.g. sort([...concepts.value]) or
concepts.value.slice() before sort) and likewise make a copy of
props.observation.concepts before sorting, so you only compare sorted copies and
do not mutate the reactive concepts array during hasChanges or when building
updates in handleSave (update references: hasChanges computed, handleSave
function, concepts.value, and props.observation.concepts).
In `@ui/src/components/search/SearchBar.vue`:
- Line 17: localValue is only initialized once from props.modelValue and doesn't
follow external updates, breaking v-model; add a watcher on props.modelValue in
SearchBar.vue that updates the ref localValue whenever props.modelValue changes
(e.g., watch(() => props.modelValue, v => localValue.value = v ?? '')), and
ensure existing input handlers continue to emit 'update:modelValue' so two-way
binding remains correct.
In `@ui/src/composables/useAuth.ts`:
- Around line 20-27: Функция login() сейчас выполняет fetch напрямую and может
выбрасывать сетевые исключения, в отличие от checkAuth() и logout(); оберни
вызов fetch в try/catch внутри login(), в catch выставляй authenticated.value =
false и возвращай false, а в try проверяй res.ok как сейчас; используй те же
переменные/URL (login(), authenticated.value, '/api/auth/login') чтобы поведение
composable при сетевых ошибках стало согласованным и не приводило к
необработанным rejection в UI.
In `@ui/src/composables/usePagination.ts`:
- Around line 30-47: The bug is that an earlier request’s finally block can flip
loading.value to false after a newer fetch started; fix fetchPage by
snapshotting the current abortController (or a requestId) immediately after
creating it and before awaiting options.fetchFn, and in the catch/finally only
update loading.value (and items/total/error) if the snapshot matches the current
abortController (i.e., the request is still the active one); reference
fetchPage, abortController, loading, items, total, and error when making the
conditional guard so only the active request mutates shared state.
In `@ui/src/composables/useSearch.ts`:
- Around line 49-57: The frontend is sending the literal 'all' as project which
the backend treats as a real project name; change the project param in the
searchObservations call from p || 'all' to p || '' (use empty string for all
projects). Also normalize totalCount handling: after receiving response from
searchObservations (used by context and decision searches) set totalCount to
response.total_count when that field exists, otherwise fall back to
response.observations.length so pagination-aware responses are respected; update
references around results.value, intent.value and totalCount.value accordingly.
In `@ui/src/utils/api.ts`:
- Around line 378-387: The function fetchObservationsPaginated omits valid falsy
values (0) because it checks params.limit and params.offset with truthiness;
change those checks to explicit undefined checks (e.g., params.limit !==
undefined and params.offset !== undefined) so offset=0 and limit=0 are included
in the URLSearchParams, leaving the project check as-is and continuing to call
fetchWithRetry with the constructed query string.
In `@ui/src/views/ObservationsView.vue`:
- Around line 41-53: The computed filteredObservations is applying
currentType/currentConcept after pagination which yields wrong totals and
availableConcepts; change the logic so filters are applied before pagination (or
pass currentType/currentConcept into the data fetch query) and ensure
total/availableConcepts are computed from the filtered dataset; also reset the
pagination offset whenever currentType or currentConcept changes. Update the
computed/requests around filteredObservations, observations, total, Pagination,
availableConcepts and the offset reset in the same places referenced (also
update the similar logic at the other occurrences noted around the 64-74 and
102-108 blocks).
In `@ui/src/views/SearchView.vue`:
- Around line 23-26: The search state is only updating local query
(handleSearch) while reads come from route.query.q, causing stale/losing state;
change handleSearch to update the URL (use router.push/replace with { query: { q
} } or remove q when empty) instead of only setting query.value, and add a
watcher on route.query.q (or derive query.value from route.query.q) to call
search() whenever the route param changes and to clear local state/results when
q is absent; apply the same fix to the other similar handlers/blocks referenced
around lines 43-59 so the router URL is the single source of truth.
---
Nitpick comments:
In `@internal/db/gorm/migrations.go`:
- Around line 1217-1229: Add a UNIQUE constraint on the api_tokens.token_hash to
prevent hash collisions: update the CREATE TABLE statement that defines
api_tokens to mark token_hash as UNIQUE (or add a UNIQUE constraint name) and
also add a migration step to enforce uniqueness on existing databases by
creating a unique index or ALTER TABLE ... ADD CONSTRAINT (use IF NOT EXISTS
semantics where supported). Locate the SQL string in migrations.go that builds
the api_tokens table (the CREATE TABLE IF NOT EXISTS api_tokens ... block) and
modify it, and add an accompanying migration function/step in the same
migrations list to create the unique index/constraint for existing data.
In `@internal/worker/handlers_analytics.go`:
- Around line 126-135: Replace the manual O(n²) selection-sort loop over
topConcepts with sort.Slice: import "sort", call sort.Slice(topConcepts, func(i,
j int) bool { return topConcepts[i].count > topConcepts[j].count }), then
truncate to topConcepts = topConcepts[:min(10, len(topConcepts))] (or keep the
existing slice-length guard) so the slice is sorted descending by count and then
limited to 10; update any references to topConcepts in handlers_analytics.go
accordingly.
- Around line 53-58: When the "days" query param is present but invalid, don't
silently fall back to 30; validate r.URL.Query().Get("days") using strconv.Atoi
and if parsing fails or the parsed value is <=0 or >365, write a 400 Bad Request
response (e.g., via http.Error or similar) describing the invalid "days" value
instead of assigning the default to the days variable; otherwise set days to the
parsed value. Ensure this change is applied where the current logic around days,
r.URL.Query().Get("days") and strconv.Atoi is implemented so clients receive
explicit feedback for bad input.
In `@internal/worker/handlers_auth.go`:
- Around line 358-370: The containsDuplicateKey function currently does manual
substring scanning; replace the manual loops with calls to strings.Contains for
each pattern by iterating over the slice {"duplicate key","23505","UNIQUE
constraint"} and returning true if any strings.Contains(msg, s) is true; also
add the "strings" import (or include it in the existing import block) and remove
the manual index-based logic to simplify and rely on the stdlib implementation.
In `@internal/worker/handlers_tags.go`:
- Around line 53-63: The current check rejects an empty req.Tags for all
actions, preventing clearing all tags via the "set" action; update the
validation in the handler that currently checks len(req.Tags) == 0 so that it
only returns a 400 when tags are empty for actions other than "set" (i.e., allow
an empty slice when req.Action == "set"), or if the empty-for-set behavior was
intentional, add a clear inline comment next to that validation mentioning the
decision; reference req.Tags and req.Action in your change.
In `@internal/worker/service.go`:
- Around line 1582-1584: Auth endpoints / handlers handleAuthLogin and
handleAuthLogout are intentionally exposed outside the requireReady group and
must remain independent of DB readiness (they only use s.tokenAuth); update the
codebase to enforce this: add a clear code comment near the router registrations
and the handlers (handleAuthLogin, handleAuthLogout) documenting the invariant
that these handlers must not perform blocking DB calls, and if you add
logging/audit in the future, perform it asynchronously (e.g., fire-and-forget
goroutine or enqueue to an async logger/queue) so it never waits for
requireReady or DB initialization; alternatively, if synchronous DB audit is
required, move those endpoints into the requireReady group—choose one approach
and document it next to s.tokenAuth and the router registrations.
In `@internal/worker/token_stats.go`:
- Around line 70-72: The flushTokenStats function currently uses
context.Background() which ignores cancellation/timeouts; change flushTokenStats
to accept a context.Context parameter and pass that ctx into
store.BatchIncrementStats (replace context.Background() with ctx), then update
all callers of flushTokenStats to forward their available context (e.g., request
or service ctx) so the batch flush respects cancellation and deadlines;
reference functions: flushTokenStats and store.BatchIncrementStats.
In `@ui/src/components/layout/AppSidebar.vue`:
- Around line 76-90: The links rendered by the router-link v-for (using
navItems, isActive, collapsed, item.label and item.icon) lack accessible names
when collapsed; update the router-link and other icon-only buttons (the blocks
referenced at lines 96-112 and 115-143) to provide an explicit accessible name
by adding aria-label="{item.label}" when collapsed is true (or include an
sr-only <span> with item.label that is conditionally rendered only when
collapsed) so screen readers receive a clear name while preserving the visual
collapsed icon-only UI.
In `@ui/src/components/layout/ConfirmDialog.vue`:
- Around line 18-50: The modal lacks basic accessibility: add dialog semantics
and Esc handling. On the dialog container element (the div with classes
"relative glass ...", inside ConfirmDialog.vue) add role="dialog",
aria-modal="true", aria-labelledby pointing to the title ID and aria-describedby
pointing to the message ID; give the <h3> (title) and the <p> (message) stable
IDs (e.g., titleId/messageId). Also add an Esc key handler (e.g.,
`@keydown.escape.window` or a keydown listener that emits('cancel')) so pressing
Esc closes the dialog, and ensure the dialog is focusable (tabindex="-1") and
focused when opened (e.g., focus it when show becomes true) to support
keyboard/assistive navigation.
In `@ui/src/components/layout/Pagination.vue`:
- Around line 69-75: The previous/next navigation buttons in Pagination.vue (the
<button> elements that call goToPage and check currentPage) lack aria-labels for
screen readers; update both button elements (the one with
`@click`="goToPage(currentPage - 1)" and the corresponding next button with
`@click`="goToPage(currentPage + 1)") to include descriptive aria-label attributes
like "Previous page" and "Next page" respectively so assistive technologies can
announce their purpose.
In `@ui/src/composables/useObservation.ts`:
- Around line 39-64: The save/archive/feedback functions lack AbortSignal
handling and should mirror the load pattern: create an AbortController at the
start of save (and similarly in archive and feedback), pass controller.signal
into the API call (e.g., updateObservation(id, updates, { signal }) or
updateObservation({ id, updates, signal }) matching the API signature), register
onUnmounted(() => controller.abort()) (or reuse a per-operation controller and
abort previous controller before creating a new one) so requests are cancelled
when the component unmounts, and handle abort errors in the existing catch block
without setting a user-facing error; reference the save function and its call to
updateObservation as well as the existing load implementation for the exact
abort/cleanup pattern.
In `@ui/src/router/index.ts`:
- Around line 4-71: The routes array lacks a catch-all 404 route; add a final
route entry to routes (after existing entries) with a path like
'/:pathMatch(.*)*', name 'not-found' and a component that renders your NotFound
view (e.g., lazy-import '@/views/NotFoundView.vue') or redirect to an existing
route; ensure the route is appended at the end so Vue Router will use it for
unmatched URLs.
In `@ui/src/views/LoginView.vue`:
- Around line 29-30: The catch block currently swallows the exception and only
sets error.value; update the catch to accept the error (e) and log it (e.g.,
console.error) for debugging while keeping the user-facing assignment to
error.value; locate the catch associated with the login flow in LoginView.vue
where error.value = 'Connection error. Is the server running?' and change it to
catch (e) { console.error('Login connection error', e); error.value =
'Connection error. Is the server running?'; }.
In `@ui/src/views/ObservationDetailView.vue`:
- Around line 269-305: The template calls splitPath(file) twice per file; cache
its result by creating computed properties (e.g., filesReadSplit and
filesModifiedSplit) that map observation.files_read and
observation.files_modified to arrays of objects { orig: file, root: ..., path:
... } using splitPath once, then update the v-for loops (currently iterating
observation.files_read and observation.files_modified) to iterate the new
computed arrays and reference file.root/file.path (and use file.orig for :key
and :title) so splitPath is called only once per file; alternatively, use Vue 3
v-memo in the same v-for if you prefer to keep logic in-template.
- Line 2: Remove the unused onUnmounted import from the top-level import
statement in ObservationDetailView.vue; the file only uses onBeforeUnmount (and
other imports like ref, computed, onMounted), so update the import list to
exclude onUnmounted to eliminate dead code and lint warnings.
🪄 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: 4bb3a25a-2b93-49ed-b39f-2d1baa8f558d
📒 Files selected for processing (47)
internal/db/gorm/migrations.gointernal/db/gorm/models.gointernal/db/gorm/token_store.gointernal/worker/handlers_analytics.gointernal/worker/handlers_auth.gointernal/worker/handlers_maintenance.gointernal/worker/handlers_sessions_rest.gointernal/worker/handlers_tags.gointernal/worker/handlers_vault.gointernal/worker/middleware.gointernal/worker/service.gointernal/worker/token_stats.goui/package.jsonui/src/App.vueui/src/components/layout/AppHeader.vueui/src/components/layout/AppSidebar.vueui/src/components/layout/ConfirmDialog.vueui/src/components/layout/EmptyState.vueui/src/components/layout/Pagination.vueui/src/components/observation/ObservationEditor.vueui/src/components/search/SearchBar.vueui/src/components/search/SearchResults.vueui/src/composables/index.tsui/src/composables/useAuth.tsui/src/composables/useObservation.tsui/src/composables/usePagination.tsui/src/composables/useSearch.tsui/src/main.tsui/src/router/index.tsui/src/types/index.tsui/src/types/observation.tsui/src/types/search.tsui/src/utils/api.tsui/src/views/AnalyticsView.vueui/src/views/GraphView.vueui/src/views/HomeView.vueui/src/views/LoginView.vueui/src/views/LogsView.vueui/src/views/ObservationDetailView.vueui/src/views/ObservationsView.vueui/src/views/PatternsView.vueui/src/views/SearchView.vueui/src/views/SessionsView.vueui/src/views/SystemView.vueui/src/views/TokensView.vueui/src/views/VaultView.vueui/tailwind.config.js
Comment on lines
+63
to
+77
| // Revoke marks a token as revoked. | ||
| func (s *TokenStore) Revoke(ctx context.Context, id string) error { | ||
| now := time.Now() | ||
| result := s.db.WithContext(ctx). | ||
| Model(&APIToken{}). | ||
| Where("id = ?", id). | ||
| Updates(map[string]interface{}{ | ||
| "revoked": true, | ||
| "revoked_at": &now, | ||
| }) | ||
| if result.RowsAffected == 0 { | ||
| return gorm.ErrRecordNotFound | ||
| } | ||
| return result.Error | ||
| } |
There was a problem hiding this comment.
Исправьте порядок проверки ошибки в Revoke.
Текущая логика сначала проверяет RowsAffected, а затем возвращает result.Error. Если произойдёт ошибка БД (например, сетевая), но RowsAffected == 0, метод вернёт gorm.ErrRecordNotFound вместо реальной ошибки.
🐛 Предлагаемое исправление
func (s *TokenStore) Revoke(ctx context.Context, id string) error {
now := time.Now()
result := s.db.WithContext(ctx).
Model(&APIToken{}).
Where("id = ?", id).
Updates(map[string]interface{}{
"revoked": true,
"revoked_at": &now,
})
+ if result.Error != nil {
+ return result.Error
+ }
if result.RowsAffected == 0 {
return gorm.ErrRecordNotFound
}
- return result.Error
+ return nil
}📝 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
| // Revoke marks a token as revoked. | |
| func (s *TokenStore) Revoke(ctx context.Context, id string) error { | |
| now := time.Now() | |
| result := s.db.WithContext(ctx). | |
| Model(&APIToken{}). | |
| Where("id = ?", id). | |
| Updates(map[string]interface{}{ | |
| "revoked": true, | |
| "revoked_at": &now, | |
| }) | |
| if result.RowsAffected == 0 { | |
| return gorm.ErrRecordNotFound | |
| } | |
| return result.Error | |
| } | |
| // Revoke marks a token as revoked. | |
| func (s *TokenStore) Revoke(ctx context.Context, id string) error { | |
| now := time.Now() | |
| result := s.db.WithContext(ctx). | |
| Model(&APIToken{}). | |
| Where("id = ?", id). | |
| Updates(map[string]interface{}{ | |
| "revoked": true, | |
| "revoked_at": &now, | |
| }) | |
| if result.Error != nil { | |
| return result.Error | |
| } | |
| if result.RowsAffected == 0 { | |
| return gorm.ErrRecordNotFound | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/db/gorm/token_store.go` around lines 63 - 77, In TokenStore.Revoke,
the code checks result.RowsAffected before result.Error which can hide real DB
errors; change the order in the Revoke method (function TokenStore.Revoke) to
first inspect result.Error and return it if non-nil, then check
result.RowsAffected and return gorm.ErrRecordNotFound when RowsAffected == 0,
leaving the final return as nil (or result.Error if kept) so that real database
errors from the Updates call on APIToken are not masked.
Comment on lines
+95
to
+102
| http.SetCookie(w, &http.Cookie{ | ||
| Name: sessionCookieName, | ||
| Value: cookieValue, | ||
| Path: "/", | ||
| MaxAge: sessionMaxAge, | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| }) |
There was a problem hiding this comment.
Добавьте флаг Secure для cookie в production.
Cookie устанавливается без флага Secure, что позволяет передавать его по HTTP. В production-окружении с HTTPS это создаёт риск перехвата cookie при случайном HTTP-запросе.
🔒 Предлагаемое исправление
http.SetCookie(w, &http.Cookie{
Name: sessionCookieName,
Value: cookieValue,
Path: "/",
MaxAge: sessionMaxAge,
HttpOnly: true,
+ Secure: r.TLS != nil, // Set Secure flag when served over HTTPS
SameSite: http.SameSiteStrictMode,
})Альтернативно, можно использовать переменную окружения для определения production-режима.
📝 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
| http.SetCookie(w, &http.Cookie{ | |
| Name: sessionCookieName, | |
| Value: cookieValue, | |
| Path: "/", | |
| MaxAge: sessionMaxAge, | |
| HttpOnly: true, | |
| SameSite: http.SameSiteStrictMode, | |
| }) | |
| http.SetCookie(w, &http.Cookie{ | |
| Name: sessionCookieName, | |
| Value: cookieValue, | |
| Path: "/", | |
| MaxAge: sessionMaxAge, | |
| HttpOnly: true, | |
| Secure: r.TLS != nil, // Set Secure flag when served over HTTPS | |
| SameSite: http.SameSiteStrictMode, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/worker/handlers_auth.go` around lines 95 - 102, The cookie is set
without the Secure flag; update the http.SetCookie call that creates the cookie
(referencing sessionCookieName, cookieValue, sessionMaxAge, and SameSite:
http.SameSiteStrictMode) to set Secure: true when running in production (e.g.
check an existing isProd/env var or add one derived from an env var) so cookies
are only sent over HTTPS in production; ensure the boolean is conditional so
local/dev HTTP remains unaffected.
Comment on lines
+118
to
+125
| http.SetCookie(w, &http.Cookie{ | ||
| Name: sessionCookieName, | ||
| Value: "", | ||
| Path: "/", | ||
| MaxAge: -1, | ||
| HttpOnly: true, | ||
| SameSite: http.SameSiteStrictMode, | ||
| }) |
There was a problem hiding this comment.
Добавьте флаг Secure для очистки cookie.
Аналогично login, logout также должен устанавливать флаг Secure для консистентности.
🔒 Предлагаемое исправление
http.SetCookie(w, &http.Cookie{
Name: sessionCookieName,
Value: "",
Path: "/",
MaxAge: -1,
HttpOnly: true,
+ Secure: r.TLS != nil,
SameSite: http.SameSiteStrictMode,
})📝 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
| http.SetCookie(w, &http.Cookie{ | |
| Name: sessionCookieName, | |
| Value: "", | |
| Path: "/", | |
| MaxAge: -1, | |
| HttpOnly: true, | |
| SameSite: http.SameSiteStrictMode, | |
| }) | |
| http.SetCookie(w, &http.Cookie{ | |
| Name: sessionCookieName, | |
| Value: "", | |
| Path: "/", | |
| MaxAge: -1, | |
| HttpOnly: true, | |
| Secure: r.TLS != nil, | |
| SameSite: http.SameSiteStrictMode, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/worker/handlers_auth.go` around lines 118 - 125, The cookie deletion
in the logout flow uses http.SetCookie with Name sessionCookieName but omits the
Secure flag; update the http.SetCookie call (the cookie literal used to clear
sessionCookieName) to include Secure: true so the cleared cookie matches the
login/logout cookie attributes (SameSite, HttpOnly, Path, MaxAge) for
consistency and security.
Comment on lines
+36
to
+41
| if r.Body != nil && r.ContentLength > 0 { | ||
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil { | ||
| http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest) | ||
| return | ||
| } | ||
| } |
There was a problem hiding this comment.
Проверка ContentLength > 0 может пропустить валидные запросы с chunked transfer encoding.
При использовании chunked transfer encoding r.ContentLength будет равен -1, что приведёт к пропуску парсинга тела запроса, даже если оно содержит валидный JSON. Рекомендуется убрать проверку ContentLength или использовать другой подход.
🛡️ Предлагаемое исправление
var req consolidationRequest
// Body is optional; default to "all"
- if r.Body != nil && r.ContentLength > 0 {
- if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+ if r.Body != nil {
+ if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err.Error() != "EOF" {
http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
return
}
}Альтернативный вариант — использовать io.EOF для проверки пустого тела:
if r.Body != nil {
decoder := json.NewDecoder(r.Body)
if err := decoder.Decode(&req); err != nil && err != io.EOF {
http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest)
return
}
}📝 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
| if r.Body != nil && r.ContentLength > 0 { | |
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil { | |
| http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest) | |
| return | |
| } | |
| } | |
| if r.Body != nil { | |
| if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err.Error() != "EOF" { | |
| http.Error(w, "invalid JSON body: "+err.Error(), http.StatusBadRequest) | |
| return | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/worker/handlers_maintenance.go` around lines 36 - 41, The handler
currently skips JSON parsing when r.ContentLength <= 0, which breaks chunked
requests; remove the ContentLength check and always attempt decoding from r.Body
(if r.Body != nil), using json.NewDecoder(r.Body).Decode(&req) and treat io.EOF
as non-error (i.e., only return HTTP 400 when err != nil && err != io.EOF) so
chunked transfer-encoded bodies are parsed correctly.
Comment on lines
+32
to
+37
| limit := 20 | ||
| if val := r.URL.Query().Get("limit"); val != "" { | ||
| if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 { | ||
| limit = parsed | ||
| } | ||
| } |
There was a problem hiding this comment.
Ограничьте верхнюю границу limit в обоих endpoint’ах.
Сейчас клиент может запросить слишком большой limit, что создаёт риск перегрузки БД/памяти на API-узле.
🛠️ Вариант исправления
+const (
+ defaultListLimit = 20
+ defaultSearchLimit = 10
+ maxLimit = 100
+)
+
func (s *Service) handleListIndexedSessions(w http.ResponseWriter, r *http.Request) {
- limit := 20
+ limit := defaultListLimit
if val := r.URL.Query().Get("limit"); val != "" {
if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 {
+ if parsed > maxLimit {
+ parsed = maxLimit
+ }
limit = parsed
}
}
@@
- limit := 10
+ limit := defaultSearchLimit
if val := r.URL.Query().Get("limit"); val != "" {
if parsed, err := strconv.Atoi(val); err == nil && parsed > 0 {
+ if parsed > maxLimit {
+ parsed = maxLimit
+ }
limit = parsed
}
}Also applies to: 117-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/worker/handlers_sessions_rest.go` around lines 32 - 37, The limit
parsing currently trusts any positive client-provided value; add an explicit
upper bound (e.g. define a maxLimit constant) and clamp the parsed value to that
maximum when assigning to limit in both places where the block appears (the
limit variable parsing in handlers_sessions_rest.go and the similar block at the
other endpoint around lines 117-122). Update the logic that parses strconv.Atoi
to: validate parsed > 0, then set limit = min(parsed, maxLimit) (or set limit =
maxLimit if parsed > maxLimit) so the API cannot accept an unbounded large
limit.
Comment on lines
+30
to
+47
| async function fetchPage() { | ||
| abortController?.abort() | ||
| abortController = new AbortController() | ||
|
|
||
| loading.value = true | ||
| error.value = null | ||
|
|
||
| try { | ||
| const result = await options.fetchFn(pageSize, offset.value, abortController.signal) | ||
| items.value = result.items | ||
| total.value = result.total | ||
| } catch (err) { | ||
| if (err instanceof Error && err.name === 'AbortError') return | ||
| error.value = err instanceof Error ? err.message : 'Failed to load data' | ||
| console.error('[usePagination] Fetch error:', err) | ||
| } finally { | ||
| loading.value = false | ||
| } |
There was a problem hiding this comment.
Старый aborted-запрос может сбросить loading у нового.
Если fetchPage() вызывается дважды подряд, первый запрос дойдёт до finally уже после старта второго и выставит loading = false, хотя новая загрузка ещё идёт. Для общего composable это даст ложное “idle” состояние и мигание UI.
💡 Возможный фикс
async function fetchPage() {
abortController?.abort()
- abortController = new AbortController()
+ const controller = new AbortController()
+ abortController = controller
loading.value = true
error.value = null
try {
- const result = await options.fetchFn(pageSize, offset.value, abortController.signal)
+ const result = await options.fetchFn(pageSize, offset.value, controller.signal)
+ if (abortController !== controller) return
items.value = result.items
total.value = result.total
} catch (err) {
if (err instanceof Error && err.name === 'AbortError') return
+ if (abortController !== controller) return
error.value = err instanceof Error ? err.message : 'Failed to load data'
console.error('[usePagination] Fetch error:', err)
} finally {
- loading.value = false
+ if (abortController === controller) {
+ loading.value = false
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/composables/usePagination.ts` around lines 30 - 47, The bug is that an
earlier request’s finally block can flip loading.value to false after a newer
fetch started; fix fetchPage by snapshotting the current abortController (or a
requestId) immediately after creating it and before awaiting options.fetchFn,
and in the catch/finally only update loading.value (and items/total/error) if
the snapshot matches the current abortController (i.e., the request is still the
active one); reference fetchPage, abortController, loading, items, total, and
error when making the conditional guard so only the active request mutates
shared state.
Comment on lines
+49
to
+57
| } else { | ||
| const response = await searchObservations( | ||
| { query: q, project: p || 'all', limit: 50 }, | ||
| abortController.signal | ||
| ) | ||
| results.value = response.observations | ||
| totalCount.value = response.observations.length | ||
| intent.value = response.intent || '' | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверяем, как бэкенд обрабатывает project='all'
rg -n -A 5 'context/search|handleContextSearch' --type=goRepository: thebtf/engram
Length of output: 1790
🏁 Script executed:
cat -n ui/src/composables/useSearch.ts | head -100Repository: thebtf/engram
Length of output: 3068
🏁 Script executed:
# Check if there's any special handling of 'all' project value in the backend
rg -n "project.*=.*all|'all'.*project|\"all\".*project" --type=go -B 2 -A 2Repository: thebtf/engram
Length of output: 759
🏁 Script executed:
# Look at the complete backend handlers for context search
sed -n '40,150p' internal/worker/handlers_context.go | cat -nRepository: thebtf/engram
Length of output: 4774
🏁 Script executed:
# Find ValidateProjectName implementation
rg -n "func.*ValidateProjectName" --type=go -A 15Repository: thebtf/engram
Length of output: 2006
🏁 Script executed:
# Check the API types/response for searchObservations
rg -n "SearchResult|searchObservations|response\.observations" ui/src/types --type=ts -B 2 -A 5Repository: thebtf/engram
Length of output: 1047
🏁 Script executed:
# Check the API utils to see the response structure
grep -n "searchObservations\|searchDecisions" ui/src/utils/api.ts -A 10Repository: thebtf/engram
Length of output: 575
🏁 Script executed:
# Check projectNamePattern to see if 'all' would be accepted
rg -n "projectNamePattern" internal/worker/middleware.go -B 5Repository: thebtf/engram
Length of output: 515
🏁 Script executed:
# Verify the complete ValidateProjectName function
sed -n '451,475p' internal/worker/middleware.goRepository: thebtf/engram
Length of output: 633
🏁 Script executed:
# Check if 'all' is used elsewhere or if there's special handling for it in search logic
rg -n "\"all\"|'all'" internal/worker/handlers_context.go -B 3 -A 3Repository: thebtf/engram
Length of output: 39
🏁 Script executed:
# Check what the actual backend returns for context search - does it calculate total or just return limited results?
sed -n '90,200p' internal/worker/handlers_context.go | grep -A 20 "allVectorResults\|json.NewEncoder"Repository: thebtf/engram
Length of output: 2141
🏁 Script executed:
# Check how the backend filters observations by project - does it handle 'all' specially?
rg -n "where.*project|project.*filter|ExtractObservationIDs" internal/worker/handlers_context.go -A 3Repository: thebtf/engram
Length of output: 933
🏁 Script executed:
# Check the vector search helper to see if 'all' is handled
rg -n "ExtractObservationIDs|BuildWhereFilter" internal/vector/ --type=go -A 10Repository: thebtf/engram
Length of output: 1945
🏁 Script executed:
# Check if there's any special handling or documentation for searching without project filter
rg -n "all projects|project.*filter|optional.*project" internal/worker/ --type=go -B 2 -A 2Repository: thebtf/engram
Length of output: 3949
🏁 Script executed:
# Check git history or comments about the 'all' literal value
git log --oneline -n 20 -- ui/src/composables/useSearch.ts 2>/dev/null || echo "Git history not available"Repository: thebtf/engram
Length of output: 105
Критическая ошибка: 'all' передаётся как имя проекта, но бэкенд требует пустую строку для поиска по всем проектам.
-
Строка 51: Фронтенд отправляет литеральное значение
'all'в качестве проекта, когда значение пусто. Однако бэкенд требует пустую строку для поиска по всем проектам (см. документацию service.go:1856 и логику handlers_context.go:42-44). Значение'all'будет обработано как имя конкретного проекта, что сломает поиск контекста. Исправить наp || ''. -
Несогласованность
totalCount: типы ответа различаются —ContextSearchResponseне содержит полеtotal_count, аDecisionSearchResponseсодержит. Для контекстного поискаtotalCountустанавливается какobservations.length(количество возвращённых результатов), для decision-поиска используетсяresponse.total_count. Если бэкенд использует пагинацию с лимитом 50,totalCountбудет показывать только полученные элементы, а не общее количество.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/composables/useSearch.ts` around lines 49 - 57, The frontend is
sending the literal 'all' as project which the backend treats as a real project
name; change the project param in the searchObservations call from p || 'all' to
p || '' (use empty string for all projects). Also normalize totalCount handling:
after receiving response from searchObservations (used by context and decision
searches) set totalCount to response.total_count when that field exists,
otherwise fall back to response.observations.length so pagination-aware
responses are respected; update references around results.value, intent.value
and totalCount.value accordingly.
Comment on lines
+378
to
+387
| export async function fetchObservationsPaginated( | ||
| params: { limit?: number; offset?: number; project?: string }, | ||
| signal?: AbortSignal | ||
| ): Promise<ObservationsResponse> { | ||
| const searchParams = new URLSearchParams() | ||
| if (params.limit) searchParams.append('limit', String(params.limit)) | ||
| if (params.offset) searchParams.append('offset', String(params.offset)) | ||
| if (params.project) searchParams.append('project', params.project) | ||
| return fetchWithRetry<ObservationsResponse>(`${API_BASE}/observations?${searchParams}`, { signal }) | ||
| } |
There was a problem hiding this comment.
offset=0 не будет добавлен в параметры запроса.
Проверка if (params.offset) вернёт false для offset=0, что является валидным значением для первой страницы. Аналогично для limit=0.
🐛 Предлагаемое исправление
export async function fetchObservationsPaginated(
params: { limit?: number; offset?: number; project?: string },
signal?: AbortSignal
): Promise<ObservationsResponse> {
const searchParams = new URLSearchParams()
- if (params.limit) searchParams.append('limit', String(params.limit))
- if (params.offset) searchParams.append('offset', String(params.offset))
+ if (params.limit !== undefined) searchParams.append('limit', String(params.limit))
+ if (params.offset !== undefined) searchParams.append('offset', String(params.offset))
if (params.project) searchParams.append('project', params.project)
return fetchWithRetry<ObservationsResponse>(`${API_BASE}/observations?${searchParams}`, { signal })
}📝 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
| export async function fetchObservationsPaginated( | |
| params: { limit?: number; offset?: number; project?: string }, | |
| signal?: AbortSignal | |
| ): Promise<ObservationsResponse> { | |
| const searchParams = new URLSearchParams() | |
| if (params.limit) searchParams.append('limit', String(params.limit)) | |
| if (params.offset) searchParams.append('offset', String(params.offset)) | |
| if (params.project) searchParams.append('project', params.project) | |
| return fetchWithRetry<ObservationsResponse>(`${API_BASE}/observations?${searchParams}`, { signal }) | |
| } | |
| export async function fetchObservationsPaginated( | |
| params: { limit?: number; offset?: number; project?: string }, | |
| signal?: AbortSignal | |
| ): Promise<ObservationsResponse> { | |
| const searchParams = new URLSearchParams() | |
| if (params.limit !== undefined) searchParams.append('limit', String(params.limit)) | |
| if (params.offset !== undefined) searchParams.append('offset', String(params.offset)) | |
| if (params.project) searchParams.append('project', params.project) | |
| return fetchWithRetry<ObservationsResponse>(`${API_BASE}/observations?${searchParams}`, { signal }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/api.ts` around lines 378 - 387, The function
fetchObservationsPaginated omits valid falsy values (0) because it checks
params.limit and params.offset with truthiness; change those checks to explicit
undefined checks (e.g., params.limit !== undefined and params.offset !==
undefined) so offset=0 and limit=0 are included in the URLSearchParams, leaving
the project check as-is and continuing to call fetchWithRetry with the
constructed query string.
Comment on lines
+41
to
+53
| // Client-side filtering on paginated data | ||
| const filteredObservations = computed(() => { | ||
| let items = observations.value | ||
| if (currentType.value) { | ||
| items = items.filter(o => o.type === currentType.value) | ||
| } | ||
| if (currentConcept.value) { | ||
| items = items.filter(o => | ||
| Array.isArray(o.concepts) && o.concepts.includes(currentConcept.value) | ||
| ) | ||
| } | ||
| return items | ||
| }) |
There was a problem hiding this comment.
Фильтры применяются после пагинации, поэтому список и счётчики неверны.
Сейчас type и concept режут только уже загруженную страницу, а total, Pagination и availableConcepts считаются по нефильтрованному набору. Из-за этого на одной странице легко получить пустой список, хотя совпадения есть на других страницах. Эти фильтры нужно переносить в запрос или пагинировать уже отфильтрованный набор; при смене фильтра ещё и сбрасывать offset.
Also applies to: 64-74, 102-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/views/ObservationsView.vue` around lines 41 - 53, The computed
filteredObservations is applying currentType/currentConcept after pagination
which yields wrong totals and availableConcepts; change the logic so filters are
applied before pagination (or pass currentType/currentConcept into the data
fetch query) and ensure total/availableConcepts are computed from the filtered
dataset; also reset the pagination offset whenever currentType or currentConcept
changes. Update the computed/requests around filteredObservations, observations,
total, Pagination, availableConcepts and the offset reset in the same places
referenced (also update the similar logic at the other occurrences noted around
the 64-74 and 102-108 blocks).
Comment on lines
+23
to
+26
| function handleSearch(q: string) { | ||
| query.value = q | ||
| search() | ||
| } |
There was a problem hiding this comment.
Состояние поиска сейчас расходится с URL.
Поиск из этой страницы обновляет только локальный query, а чтение идёт из route.query.q. После refresh/back пользователь теряет текущий запрос, а при переходе на /search без q остаются старые результаты. Лучше сделать URL источником истины: писать q через router и очищать состояние, когда параметр исчезает.
💡 Возможный фикс
-import { useRoute } from 'vue-router'
+import { useRoute, useRouter } from 'vue-router'
@@
const route = useRoute()
+const router = useRouter()
@@
function handleSearch(q: string) {
- query.value = q
- search()
+ const value = q.trim()
+ router.push({ name: 'search', query: value ? { q: value } : {} })
}
@@
-onMounted(() => {
- loadProjects()
- const urlQuery = route.query.q as string
- if (urlQuery) {
- query.value = urlQuery
- search()
- }
-})
+function syncFromRoute(rawQ: unknown) {
+ if (typeof rawQ === 'string' && rawQ.trim()) {
+ if (rawQ !== query.value) {
+ query.value = rawQ
+ search()
+ }
+ return
+ }
+
+ clear()
+}
+
+onMounted(() => {
+ loadProjects()
+ syncFromRoute(route.query.q)
+})
@@
-watch(() => route.query.q, (newQ) => {
- if (newQ && typeof newQ === 'string' && newQ !== query.value) {
- query.value = newQ
- search()
- }
-})
+watch(() => route.query.q, syncFromRoute)Also applies to: 43-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/views/SearchView.vue` around lines 23 - 26, The search state is only
updating local query (handleSearch) while reads come from route.query.q, causing
stale/losing state; change handleSearch to update the URL (use
router.push/replace with { query: { q } } or remove q when empty) instead of
only setting query.value, and add a watcher on route.query.q (or derive
query.value from route.query.q) to call search() whenever the route param
changes and to clear local state/results when q is absent; apply the same fix to
the other similar handlers/blocks referenced around lines 43-59 so the router
URL is the single source of truth.
5 tasks
This was referenced Mar 19, 2026
This was referenced Mar 20, 2026
This was referenced Apr 12, 2026
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.
Summary
Core dashboard content pages — observation CRUD and semantic search.
Depends on: PR #19 (frontend layout), PR #18 (REST endpoints), PR #17 (auth)
New composables
useObservation— single observation CRUD (load, save, archive, delete, feedback)usePagination— reusable pagination stateuseSearch— search query, results, decision modeNew views
ObservationsView— paginated list with project/type/concept filtersObservationDetailView— full view + inline edit (explicit save/discard, beforeunload warning)SearchView— semantic search + decision mode with rejected[] displayHomeView— enhanced with stats + recent observationsNew components
ObservationEditor— inline edit formSearchBar— global search with / hotkeySearchResults— result cards with relevance scoresTest plan
Summary by CodeRabbit
New Features
Style