fix: dashboard v1.0.2 — date bugs, analytics stats, relation detection, heartbeat config#22
Conversation
- Add safeDateFormat() and safeAbsoluteDate() helpers that return "—"
for null/undefined/invalid date values
- VaultView: use safeAbsoluteDate for credential created_at dates
- AnalyticsView: use safeDateFormat for recent queries and search misses
- Fix fetchRecentSearches to unwrap {queries:[...]} response envelope
and map Go field names (timestamp->last_used, results->count)
- Add vault encryption helper text when encryption is disabled
Without json tags, Go's encoding/json marshals PascalCase field names (e.g., TotalRequests) but the dashboard frontend expects snake_case (e.g., total_requests). Add explicit json struct tags to all fields.
Add relation.Detector that runs asynchronously after observation creation: - Vector similarity search finds top 20 candidates in same project - Heuristic classification: supersedes, fixes, explains, contradicts, evolves_from - Concept overlap (Jaccard similarity) for finer-grained relation typing - Conflict auto-detection for supersedes and contradicts relations - Non-blocking queue (256 buffer) with graceful shutdown on context cancellation - Wired into ObservationStore via RelationDetector interface (avoids circular imports) - Conditionally enabled only when embedding service and vector client are available
Add heartbeat.ingest config option (default: false) to control whether heartbeat/keep-alive tool events are sent to engram for ingestion. Low-value tool names (heartbeat, ping, status, noop, etc.) are now filtered out in the after-tool-call hook unless explicitly opted in. Bump openclaw-engram version 1.3.0 → 1.3.1.
WalkthroughДобавлена асинхронная система обнаружения связей между наблюдениями на основе векторного поиска: интерфейс RelationDetector и фоновой Detector, интеграция в ObservationStore и воркер. Дополнительно — heartbeat-конфиг и обработка heartbeat-инструментов в плагине, улучшенные форматтеры дат и изменения в UI/API-ответах, мелкие правки пакета. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ObsStore as ObservationStore
participant Detector as RelationDetector
participant Queue as RequestQueue
participant Vector as VectorClient
participant DB as Database
Client->>ObsStore: StoreObservation(obs)
ObsStore->>DB: Create observation
DB-->>ObsStore: obsID
ObsStore->>Detector: Enqueue(obsID, project)
Detector->>Queue: Push request (non-blocking)
alt queue full
Queue-->>Detector: Drop + warn
else queued
Queue-->>Detector: OK
end
Note over Detector,Queue: Фоновый цикл потребляет запросы
Detector->>Queue: Pop request
Queue-->>Detector: detectRequest
Detector->>DB: Load observation by obsID
DB-->>Detector: observation data
Detector->>Vector: Similarity search (project filter)
Vector-->>Detector: candidate IDs + similarities
Detector->>Detector: classifyRelation / buildSimilarityMap
Detector->>DB: Store ObservationRelation / Conflict records
DB-->>Detector: ack
Detector->>Detector: log completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate 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 focuses on enhancing the stability and intelligence of the system by addressing several dashboard-related bugs and introducing a foundational knowledge graph feature. It ensures a more robust user interface experience by fixing date display issues and improving analytics data consistency. Concurrently, it integrates an asynchronous relation detection mechanism to automatically identify connections and conflicts between observations, significantly advancing the system's ability to build a coherent knowledge base. Additionally, it refines data ingestion by allowing users to filter out irrelevant 'heartbeat' events from the OpenClaw plugin, leading to cleaner and more meaningful data. 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 several valuable fixes and a major new feature for relation detection. The frontend fixes for date formatting and analytics stats are well-implemented. The new heartbeat configuration provides useful filtering for noisy tool events. The core of this PR, the asynchronous relation detection in internal/relation/detector.go, is a significant addition. My review focuses on ensuring its robustness, particularly around graceful shutdown. I've identified one high-severity issue related to context propagation that could impact service shutdown, and I've provided a detailed suggestion to address it. The rest of the changes look solid.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
ui/src/utils/formatters.ts (2)
46-51: Избыточная тернарная операция.На строке 48 тернарная операция
typeof value === 'number' ? value : valueвсегда возвращаетvalue, что избыточно. Возможно, подразумевалось преобразование Unix timestamp (в секундах) в миллисекунды.♻️ Предлагаемое исправление
export function safeDateFormat(value: string | number | null | undefined): string { if (value === null || value === undefined || value === '' || value === 0) return '\u2014' - const date = new Date(typeof value === 'number' ? value : value) + const date = new Date(value) if (isNaN(date.getTime())) return '\u2014' return formatRelativeTime(date.getTime()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/formatters.ts` around lines 46 - 51, The ternary in safeDateFormat always returns value; update the date construction to handle numeric Unix timestamps correctly: when value is a number treat it as seconds and convert to milliseconds (value * 1000) before passing to new Date, otherwise pass the string value directly; modify the new Date call inside safeDateFormat (referencing safeDateFormat and formatRelativeTime) to use this conversion and remove the redundant ternary.
56-61: Аналогичная избыточная тернарная операция.Та же проблема на строке 58 — тернарная операция не выполняет никакого преобразования.
♻️ Предлагаемое исправление
export function safeAbsoluteDate(value: string | number | null | undefined): string { if (value === null || value === undefined || value === '' || value === 0) return '\u2014' - const date = new Date(typeof value === 'number' ? value : value) + const date = new Date(value) if (isNaN(date.getTime())) return '\u2014' return date.toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: 'numeric' }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/formatters.ts` around lines 56 - 61, The ternary in safeAbsoluteDate is redundant (new Date(typeof value === 'number' ? value : value)); update the Date construction to pass value directly (e.g., new Date(value)) so that the date variable is created correctly without the no-op ternary. Keep the existing early-return checks and isNaN(date.getTime()) logic unchanged.internal/relation/detector.go (2)
328-351: Потенциальная проблема с дубликатами вb.Concepts.Расчёт Jaccard предполагает, что
b.Conceptsне содержит дубликатов. Если дубликаты есть,intersectionможет быть подсчитан неправильно (один элемент изsetможет быть найден несколько раз).♻️ Предложение: использовать set для второго массива
func conceptOverlap(a, b *models.Observation) float64 { if len(a.Concepts) == 0 || len(b.Concepts) == 0 { return 0 } - set := make(map[string]bool, len(a.Concepts)) + setA := make(map[string]bool, len(a.Concepts)) for _, c := range a.Concepts { - set[c] = true + setA[c] = true } + setB := make(map[string]bool, len(b.Concepts)) + for _, c := range b.Concepts { + setB[c] = true + } + var intersection int - for _, c := range b.Concepts { - if set[c] { + for c := range setB { + if setA[c] { intersection++ } } - union := len(set) + len(b.Concepts) - intersection + union := len(setA) + len(setB) - intersection if union == 0 { return 0 } return float64(intersection) / float64(union) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/relation/detector.go` around lines 328 - 351, The Jaccard calc in conceptOverlap can over-count when b.Concepts contains duplicates; change the logic to treat both a.Concepts and b.Concepts as sets: build setA (existing map[string]bool) and build setB (map[string]bool) from b.Concepts, compute intersection by counting keys present in both maps, compute union as len(setA)+len(setB)-intersection, and return intersection/union; adjust references in the function conceptOverlap and models.Observation usage accordingly.
53-70: Отсутствует проверка nil-параметров в конструкторе.Комментарий предупреждает, что передача
nilвызовет панику во время выполнения, но проверка не выполняется. Лучше проваливаться рано с понятным сообщением.🛡️ Предложение: добавить проверку nil
func NewDetector( embedSvc *embedding.Service, vectorClient vector.Client, relationStore *gorm.RelationStore, conflictStore *gorm.ConflictStore, observationStore *gorm.ObservationStore, ) *Detector { + if vectorClient == nil || relationStore == nil || conflictStore == nil || observationStore == nil { + panic("relation.NewDetector: required dependency is nil") + } return &Detector{ embedSvc: embedSvc,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/relation/detector.go` around lines 53 - 70, The NewDetector constructor currently accepts required dependencies but does not validate them, causing later panics; update NewDetector to validate that embedSvc, vectorClient, relationStore, conflictStore, and observationStore are non-nil and fail fast with a clear panic or error message naming the offending parameter(s). Locate the NewDetector function and add nil checks for each parameter (embedSvc, vectorClient, relationStore, conflictStore, observationStore) and return/raise an explicit error/panic (e.g., "NewDetector: nil <ParamName>") before constructing the Detector and creating the queue (queueSize/detectRequest remain unchanged).internal/db/gorm/observation_store.go (1)
144-147: Отсутствует синхронизация при установкеrelationDetector.Метод
SetRelationDetectorустанавливает поле, которое читается вStoreObservation. Без синхронизации возможна гонка данных при параллельном доступе.Однако, если
SetRelationDetectorвызывается только один раз при инициализации (до обработки запросов), это безопасно. Рекомендуется добавить комментарий, документирующий это ограничение.📝 Предложение: добавить документацию об ограничении
// SetRelationDetector sets the async relation detector for post-creation detection. +// Must be called during initialization, before any calls to StoreObservation. func (s *ObservationStore) SetRelationDetector(d RelationDetector) { s.relationDetector = d }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/db/gorm/observation_store.go` around lines 144 - 147, SetRelationDetector currently writes the relationDetector field that StoreObservation reads without synchronization; either make access concurrency-safe or document the init-only restriction. Update SetRelationDetector and the relationDetector field comment to state that SetRelationDetector must be called only once during initialization (before any calls to StoreObservation), or alternatively protect reads/writes using a mutex/atomic (e.g., add a sync.Mutex on ObservationStore and guard relationDetector access in SetRelationDetector and StoreObservation); include references to SetRelationDetector, relationDetector, and StoreObservation so reviewers can locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/relation/detector.go`:
- Around line 312-318: The current contradiction check compares
newObs.Title.String and candidate.Title.String without verifying
sql.NullString.Valid, causing false positives when one side lacks a title;
update the logic in the contradiction block (where newObs.Type and
candidate.Type are models.ObsTypeDecision and similarity >
contradictSimilarityThreshold) to only consider titles for contradiction if both
newObs.Title.Valid and candidate.Title.Valid are true and the strings differ
(otherwise treat as non-contradictory); leave the return of
models.RelationContradicts and the similarity adjustment unchanged when both
titles are valid and different.
- Around line 43-70: The Detector struct declares and NewDetector accepts
embedSvc *embedding.Service but the field is never used (vectorClient.Query is
passed raw text and pgvector handles embedding), so remove the unused field and
constructor parameter: delete embedSvc from the Detector struct and remove the
embedSvc parameter from NewDetector (and its initialization), then update all
call sites that construct a Detector to stop passing an embedding.Service;
ensure any tests or mocks creating a Detector are adjusted accordingly. If
instead you meant to use explicit embeddings, refactor vectorClient.Query
callers to accept embeddings produced by embedSvc and wire embedSvc into
Detector; otherwise prefer removal to avoid the dead parameter.
---
Nitpick comments:
In `@internal/db/gorm/observation_store.go`:
- Around line 144-147: SetRelationDetector currently writes the relationDetector
field that StoreObservation reads without synchronization; either make access
concurrency-safe or document the init-only restriction. Update
SetRelationDetector and the relationDetector field comment to state that
SetRelationDetector must be called only once during initialization (before any
calls to StoreObservation), or alternatively protect reads/writes using a
mutex/atomic (e.g., add a sync.Mutex on ObservationStore and guard
relationDetector access in SetRelationDetector and StoreObservation); include
references to SetRelationDetector, relationDetector, and StoreObservation so
reviewers can locate the changes.
In `@internal/relation/detector.go`:
- Around line 328-351: The Jaccard calc in conceptOverlap can over-count when
b.Concepts contains duplicates; change the logic to treat both a.Concepts and
b.Concepts as sets: build setA (existing map[string]bool) and build setB
(map[string]bool) from b.Concepts, compute intersection by counting keys present
in both maps, compute union as len(setA)+len(setB)-intersection, and return
intersection/union; adjust references in the function conceptOverlap and
models.Observation usage accordingly.
- Around line 53-70: The NewDetector constructor currently accepts required
dependencies but does not validate them, causing later panics; update
NewDetector to validate that embedSvc, vectorClient, relationStore,
conflictStore, and observationStore are non-nil and fail fast with a clear panic
or error message naming the offending parameter(s). Locate the NewDetector
function and add nil checks for each parameter (embedSvc, vectorClient,
relationStore, conflictStore, observationStore) and return/raise an explicit
error/panic (e.g., "NewDetector: nil <ParamName>") before constructing the
Detector and creating the queue (queueSize/detectRequest remain unchanged).
In `@ui/src/utils/formatters.ts`:
- Around line 46-51: The ternary in safeDateFormat always returns value; update
the date construction to handle numeric Unix timestamps correctly: when value is
a number treat it as seconds and convert to milliseconds (value * 1000) before
passing to new Date, otherwise pass the string value directly; modify the new
Date call inside safeDateFormat (referencing safeDateFormat and
formatRelativeTime) to use this conversion and remove the redundant ternary.
- Around line 56-61: The ternary in safeAbsoluteDate is redundant (new
Date(typeof value === 'number' ? value : value)); update the Date construction
to pass value directly (e.g., new Date(value)) so that the date variable is
created correctly without the no-op ternary. Keep the existing early-return
checks and isNaN(date.getTime()) logic unchanged.
🪄 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: 8d67bf13-fb86-4e64-bf14-6b9164941cd8
📒 Files selected for processing (10)
internal/db/gorm/observation_store.gointernal/relation/detector.gointernal/worker/service.goplugin/openclaw-engram/package.jsonplugin/openclaw-engram/src/config.tsplugin/openclaw-engram/src/hooks/after-tool-call.tsui/src/utils/api.tsui/src/utils/formatters.tsui/src/views/AnalyticsView.vueui/src/views/VaultView.vue
|
@codex review |
…on, NullString check
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/relation/detector.go (1)
291-294: Рекомендация: добавить проверку временного порядка для supersedes.Логика supersedes предполагает, что новое наблюдение вытесняет старое. Однако при высокой конкурентности векторный поиск может вернуть наблюдение, созданное позже текущего (если оно было быстро проиндексировано). Это приведёт к семантически некорректному отношению "A supersedes B", где B на самом деле новее A.
♻️ Предложенное улучшение
// supersedes: very high similarity, same type, same project -if similarity > supersedeSimilarityThreshold && newObs.Type == candidate.Type { +if similarity > supersedeSimilarityThreshold && newObs.Type == candidate.Type && newObs.CreatedAt.After(candidate.CreatedAt) { return models.RelationSupersedes, similarity }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/relation/detector.go` around lines 291 - 294, The supersedes branch in detector.go currently only checks similarity and type; add a temporal-order check so we only return models.RelationSupersedes when the new observation is strictly newer than the candidate (e.g., compare newObs.Timestamp or CreatedAt with candidate.Timestamp/CreatedAt using After/greater-than). Update the if guarding similarity > supersedeSimilarityThreshold && newObs.Type == candidate.Type to also require newObs to be later than candidate (handle nil/zero timestamps defensively), ensuring we preserve existing symbols: newObs, candidate, supersedeSimilarityThreshold, and models.RelationSupersedes.
🤖 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/relation/detector.go`:
- Around line 76-88: The drain loop uses d.processRequest while ctx.Done() has
fired, but processRequest builds a ctx from d.parentCtx which is canceled,
causing immediate context.Canceled errors; add a new Detector method
processDrainRequest(req detectRequest) that creates an independent context via
context.WithTimeout(context.Background(), detectionTimeout) and calls d.Detect
with that fresh context, logging warnings on errors, then change the drain
branch (the case <-ctx.Done() loop) to call d.processDrainRequest(req) instead
of d.processRequest(req) so remaining items are processed with a live timeout
context.
---
Nitpick comments:
In `@internal/relation/detector.go`:
- Around line 291-294: The supersedes branch in detector.go currently only
checks similarity and type; add a temporal-order check so we only return
models.RelationSupersedes when the new observation is strictly newer than the
candidate (e.g., compare newObs.Timestamp or CreatedAt with
candidate.Timestamp/CreatedAt using After/greater-than). Update the if guarding
similarity > supersedeSimilarityThreshold && newObs.Type == candidate.Type to
also require newObs to be later than candidate (handle nil/zero timestamps
defensively), ensuring we preserve existing symbols: newObs, candidate,
supersedeSimilarityThreshold, and models.RelationSupersedes.
🪄 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: 31fcb349-c1e9-4885-87e6-cf0089e833dd
📒 Files selected for processing (2)
internal/relation/detector.gointernal/worker/service.go
✅ Files skipped from review due to trivial changes (1)
- internal/worker/service.go
| case <-ctx.Done(): | ||
| // Drain remaining items before exiting | ||
| for { | ||
| select { | ||
| case req := <-d.queue: | ||
| d.processRequest(req) | ||
| default: | ||
| log.Info().Msg("Relation detector stopped") | ||
| return | ||
| } | ||
| } | ||
| case req := <-d.queue: | ||
| d.processRequest(req) |
There was a problem hiding this comment.
Некорректный drain при завершении: родительский контекст уже отменён.
При срабатывании ctx.Done() код входит в цикл drain и вызывает processRequest(req). Однако processRequest создаёт контекст через context.WithTimeout(d.parentCtx, ...), а d.parentCtx — это тот же ctx, который уже отменён. В результате все вызовы Detect немедленно завершаются с ошибкой context.Canceled, и drain фактически не работает.
🐛 Предложенное исправление
case <-ctx.Done():
// Drain remaining items before exiting
for {
select {
case req := <-d.queue:
- d.processRequest(req)
+ d.processDrainRequest(req)
default:
log.Info().Msg("Relation detector stopped")
return
}
}Добавьте отдельный метод для drain с независимым контекстом:
// processDrainRequest handles remaining requests during shutdown with a fresh context.
func (d *Detector) processDrainRequest(req detectRequest) {
ctx, cancel := context.WithTimeout(context.Background(), detectionTimeout)
defer cancel()
if err := d.Detect(ctx, req.ObsID, req.Project); err != nil {
log.Warn().Err(err).
Int64("obs_id", req.ObsID).
Str("project", req.Project).
Msg("Relation detection failed during drain")
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/relation/detector.go` around lines 76 - 88, The drain loop uses
d.processRequest while ctx.Done() has fired, but processRequest builds a ctx
from d.parentCtx which is canceled, causing immediate context.Canceled errors;
add a new Detector method processDrainRequest(req detectRequest) that creates an
independent context via context.WithTimeout(context.Background(),
detectionTimeout) and calls d.Detect with that fresh context, logging warnings
on errors, then change the drain branch (the case <-ctx.Done() loop) to call
d.processDrainRequest(req) instead of d.processRequest(req) so remaining items
are processed with a live timeout context.
1 participant
Summary
Production bug fixes + relation detection (core knowledge graph feature).
Phase 1: Frontend Fixes
safeDateFormat()/safeAbsoluteDate()— handles null dates (no more "Invalid Date")Phase 2: Relation Detection (CRITICAL)
internal/relation/detector.go(290 lines) — async relation detectionPhase 3: Heartbeat Config
heartbeat.ingestconfig (default: false)Test plan
go build ./cmd/worker/compilesSummary by CodeRabbit
New Features
Chores