Skip to content

Commit b879327

Browse files
committed
fix inconsistencies in logging, error handling, correlation id impl
1 parent 8ea1243 commit b879327

14 files changed

Lines changed: 480 additions & 55 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Bug Fixes & Refactoring Report (Jan 5, 2026)
2+
3+
## Summary
4+
Executed the plan `docs/plans/2026-01-05-bug-fixes-inconsistencies-2.md` to standardize logging, refactor worker logic, and fix MCP error handling.
5+
6+
## Implemented Changes
7+
8+
### 1. Infrastructure - Context-Aware Logger
9+
- **New Package:** `apps/backend/internal/logger`
10+
- **Component:** `ContextHandler` (Decorator for `slog.Handler`)
11+
- **Functionality:** Automatically extracts `correlation_id` from context (via `middleware.CorrelationKey`) and injects it into every log record.
12+
- **Integration:** Wired into `apps/backend/main.go` as the default logger.
13+
14+
### 2. Observability - Adapter Logging
15+
- **Gemini Embedder:** Added `slog.DebugContext` (input stats) and `slog.ErrorContext` (failures) to `Embed`.
16+
- **Weaviate Store:** Added structured logging to `StoreChunk` and `Search` methods.
17+
- **Outcome:** Full traceability of ingestion and search operations with correlation IDs.
18+
19+
### 3. Refactor - Link Discovery Pure Function
20+
- **New File:** `apps/backend/internal/worker/link_discovery.go`
21+
- **Function:** `DiscoverLinks` (Pure function)
22+
- **Logic:** Extracted normalization, host matching, and exclusion logic from `ResultConsumer`.
23+
- **Benefit:** Improved testability (unit tests added) and separation of concerns.
24+
25+
### 4. MCP Error Handling
26+
- **Fix:** `qurio_search` (and others) now return standard JSON-RPC Error objects (`code`, `message`) instead of embedded success results with `IsError: true`.
27+
- **Standard:** Compliant with JSON-RPC 2.0.
28+
29+
## Verification
30+
- **Unit Tests:** `handler_test.go` (Logger), `link_discovery_test.go` (Worker).
31+
- **Integration Tests:** `go test ./apps/backend/features/mcp/...` passed.
32+
- **Build:** `go build ./apps/backend/...` successful.

.serena/memories/core_foundation_state.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
## Code Quality & Testing
2323
- **Backend Coverage:** High (>90%) for core adapters (`gemini`, `weaviate`) and repositories (`settings`, `source`).
24+
- **Observability:** Context-aware JSON logging standardized across Backend and Adapters.
2425
- **Frontend Quality:** Full store and component testing coverage for `jobs`, `stats`, `sources`, and `settings`.
2526
- **Standards:** STRICT adherence to `technical-constitution` (Dependency Injection, Interface-based design).
2627

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
Plan updated with clean formatting and explicit target descriptions.
2-
`qurio_list_pages` uses `source_id`.
3-
`qurio_read_page` uses `url`.
4-
Full argument guide included for `qurio_search`.
5-
6-
## Ingestion Worker
7-
- `handle_file_task` now returns `list[dict]` (standardized with `handle_web_task`) to simplify `main.py` logic.
8-
- Metadata (Title, Author, PageCount) is extracted via Docling v2 and passed to backend.
1+
- **Logging:** Implemented `apps/backend/internal/logger` package with `ContextHandler` to automatically propagate `correlation_id` from context to JSON logs.
2+
- **Worker Logic:** Link discovery logic extracted to pure function `DiscoverLinks` in `apps/backend/internal/worker/link_discovery.go` to separate I/O from business logic.
3+
- **MCP Errors:** `qurio_search` now returns standard JSON-RPC errors (code -32603) for internal failures instead of embedded text errors.

.serena/memories/mcp_tools_spec.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,9 @@ Qurio exposes its knowledge base via the Model Context Protocol (MCP).
5151

5252
## Context Propagation
5353
All tools propagate `correlationId` from the request to internal services for traceability.
54+
55+
## Error Handling
56+
Tools return standard JSON-RPC 2.0 errors for internal failures (e.g., database connection issues, search failures).
57+
- **Code:** -32603 (Internal Error)
58+
- **Message:** Human-readable error description.
59+
- **Data:** (Optional) Additional context.

apps/backend/features/mcp/handler.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -277,14 +277,8 @@ read_page(url="https://docs.stripe.com/webhooks/signatures")`,
277277
results, err := h.retriever.Search(ctx, args.Query, opts)
278278
if err != nil {
279279
slog.Error("search failed", "error", err)
280-
return &JSONRPCResponse{
281-
JSONRPC: "2.0",
282-
ID: req.ID,
283-
Result: ToolResult{
284-
Content: []ToolContent{{Type: "text", Text: "Error: " + err.Error()}},
285-
IsError: true,
286-
},
287-
}
280+
resp := makeErrorResponse(req.ID, ErrInternal, "Search failed: "+err.Error())
281+
return &resp
288282
}
289283

290284
var textResult string

apps/backend/internal/adapter/gemini/embedder.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package gemini
22

33
import (
44
"context"
5+
"log/slog"
6+
57
"github.com/google/generative-ai-go/genai"
68
"google.golang.org/api/option"
79
)
@@ -20,9 +22,11 @@ func NewEmbedder(ctx context.Context, apiKey string) (*Embedder, error) {
2022
}
2123

2224
func (e *Embedder) Embed(ctx context.Context, text string) ([]float32, error) {
25+
slog.DebugContext(ctx, "embedding content", "model", e.model, "length", len(text))
2326
em := e.client.EmbeddingModel(e.model)
2427
res, err := em.EmbedContent(ctx, genai.Text(text))
2528
if err != nil {
29+
slog.ErrorContext(ctx, "embedding failed", "error", err)
2630
return nil, err
2731
}
2832
if res.Embedding != nil {

apps/backend/internal/adapter/weaviate/store.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package weaviate
33
import (
44
"context"
55
"fmt"
6+
"log/slog"
67
"qurio/apps/backend/internal/retrieval"
78
"qurio/apps/backend/internal/worker"
89
"github.com/weaviate/weaviate-go-client/v5/weaviate"
@@ -19,6 +20,7 @@ func NewStore(client *weaviate.Client) *Store {
1920
}
2021

2122
func (s *Store) StoreChunk(ctx context.Context, chunk worker.Chunk) error {
23+
slog.DebugContext(ctx, "storing chunk", "source_id", chunk.SourceID, "chunk_index", chunk.ChunkIndex, "url", chunk.SourceURL)
2224
properties := map[string]interface{}{
2325
"content": chunk.Content,
2426
"url": chunk.SourceURL,
@@ -53,6 +55,9 @@ func (s *Store) StoreChunk(ctx context.Context, chunk worker.Chunk) error {
5355
WithProperties(properties).
5456
WithVector(chunk.Vector).
5557
Do(ctx)
58+
if err != nil {
59+
slog.ErrorContext(ctx, "failed to store chunk", "error", err, "source_id", chunk.SourceID, "chunk_index", chunk.ChunkIndex)
60+
}
5661
return err
5762
}
5863

@@ -89,6 +94,7 @@ func (s *Store) DeleteChunksBySourceID(ctx context.Context, sourceID string) err
8994
}
9095

9196
func (s *Store) Search(ctx context.Context, query string, vector []float32, alpha float32, limit int, searchFilters map[string]interface{}) ([]retrieval.SearchResult, error) {
97+
slog.DebugContext(ctx, "searching vector store", "query", query, "alpha", alpha, "limit", limit)
9298
hybrid := s.client.GraphQL().HybridArgumentBuilder().
9399
WithQuery(query).
94100
WithVector(vector).
@@ -136,6 +142,7 @@ func (s *Store) Search(ctx context.Context, query string, vector []float32, alph
136142

137143
res, err := queryBuilder.Do(ctx)
138144
if err != nil {
145+
slog.ErrorContext(ctx, "search failed", "error", err)
139146
return nil, err
140147
}
141148

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package logger
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"qurio/apps/backend/internal/middleware"
7+
)
8+
9+
type ContextHandler struct {
10+
slog.Handler
11+
}
12+
13+
func NewContextHandler(h slog.Handler) *ContextHandler {
14+
return &ContextHandler{Handler: h}
15+
}
16+
17+
func (h *ContextHandler) Handle(ctx context.Context, r slog.Record) error {
18+
if id, ok := ctx.Value(middleware.CorrelationKey).(string); ok && id != "" {
19+
r.AddAttrs(slog.String("correlation_id", id))
20+
}
21+
return h.Handler.Handle(ctx, r)
22+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package logger
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"encoding/json"
7+
"log/slog"
8+
"qurio/apps/backend/internal/middleware"
9+
"testing"
10+
)
11+
12+
func TestContextHandler_Handle(t *testing.T) {
13+
var buf bytes.Buffer
14+
jsonHandler := slog.NewJSONHandler(&buf, nil)
15+
h := NewContextHandler(jsonHandler)
16+
logger := slog.New(h)
17+
18+
ctx := context.Background()
19+
ctx = middleware.WithCorrelationID(ctx, "test-correlation-id")
20+
21+
logger.InfoContext(ctx, "test message")
22+
23+
var logMap map[string]interface{}
24+
if err := json.Unmarshal(buf.Bytes(), &logMap); err != nil {
25+
t.Fatalf("failed to unmarshal log: %v", err)
26+
}
27+
28+
if logMap["correlation_id"] != "test-correlation-id" {
29+
t.Errorf("expected correlation_id 'test-correlation-id', got %v", logMap["correlation_id"])
30+
}
31+
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package worker
2+
3+
import (
4+
"net/url"
5+
"regexp"
6+
)
7+
8+
func DiscoverLinks(sourceID, host string, links []string, currentDepth, maxDepth int, exclusions []string) []PageDTO {
9+
if currentDepth >= maxDepth {
10+
return nil
11+
}
12+
13+
var newPages []PageDTO
14+
seen := make(map[string]bool)
15+
16+
for _, link := range links {
17+
// 1. External Check
18+
linkU, err := url.Parse(link)
19+
if err != nil || linkU.Host != host {
20+
continue
21+
}
22+
23+
// Normalize: Strip Fragment
24+
linkU.Fragment = ""
25+
normalizedLink := linkU.String()
26+
27+
// 2. Exclusion Check
28+
excluded := false
29+
for _, ex := range exclusions {
30+
if matched, _ := regexp.MatchString(ex, normalizedLink); matched {
31+
excluded = true
32+
break
33+
}
34+
}
35+
if excluded {
36+
continue
37+
}
38+
39+
if seen[normalizedLink] {
40+
continue
41+
}
42+
seen[normalizedLink] = true
43+
44+
newPages = append(newPages, PageDTO{
45+
SourceID: sourceID,
46+
URL: normalizedLink,
47+
Status: "pending",
48+
Depth: currentDepth + 1,
49+
})
50+
}
51+
return newPages
52+
}

0 commit comments

Comments
 (0)