Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions .claude/rules/logging-conventions.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ The one documented exception is `MemorySearch`'s `query` field — it's small, u

## 5. Nil-Logger Safety ★

Library code — anything in `pkg/` — must accept a `nil` logger and behave silently. Two project conventions for the fallback:
Library code — anything in `pkg/` — must accept a `nil` logger and behave silently. The two project conventions for the fallback are encoded once in `internal/loghelper`; **call the helper, never inline the `if logger == nil` block**:

| Where | Fallback | Why |
| Where | Helper | Why |
|---|---|---|
| `pkg/` libraries (`mcp`, `temperature`) | `slog.New(slog.DiscardHandler)` | Tests and embedders may not want output |
| CLI-time code (`pkg/compiler` when invoked directly) | `slog.Default()` | Users running `remindb compile` want feedback |
| `pkg/` libraries (`mcp`, `temperature`) | `loghelper.OrDiscard(l)` → `slog.New(slog.DiscardHandler)` | Tests and embedders may not want output |
| CLI-time code (`pkg/compiler` when invoked directly) | `loghelper.OrDefault(l)` → `slog.Default()` | Users running `remindb compile` want feedback |

```go
// Bad — assumes the caller passed a non-nil logger
Expand All @@ -134,26 +134,28 @@ func NewServer(st *store.Store, ..., logger *slog.Logger) *Server {
return s
}

// Goodpkg/ library default
// Badinlining the fallback; it now lives in one place
func NewServer(st *store.Store, ..., logger *slog.Logger) *Server {
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
return &Server{logger: logger, ...}
}

// Good — pkg/ library default
func NewServer(st *store.Store, ..., logger *slog.Logger) *Server {
return &Server{logger: loghelper.OrDiscard(logger), ...}
}

// Good — CLI-time default (used only when the call is the entry point)
func Run(ctx context.Context, st *store.Store, opts ...Option) error {
o := applyOptions(opts...)
logger := o.logger
if logger == nil {
logger = slog.Default()
}
logger := loghelper.OrDefault(o.logger)
...
}
```

See `pkg/mcp/server.go:26-28` and `pkg/temperature/tracker.go:30-32` for the discard pattern; `pkg/compiler/compiler.go:63-66` for the default pattern.
`internal/loghelper/loghelper.go` is the canonical implementation of both conventions; every `pkg/` site resolves through it (`OrDiscard` for libraries, `OrDefault` for CLI-entry code). No `pkg/` file should still inline `if logger == nil { logger = slog.New(...) }`.

---

Expand Down
17 changes: 17 additions & 0 deletions internal/loghelper/loghelper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package loghelper

import "log/slog"

func OrDiscard(l *slog.Logger) *slog.Logger {
if l == nil {
return slog.New(slog.DiscardHandler)
}
return l
}

func OrDefault(l *slog.Logger) *slog.Logger {
if l == nil {
return slog.Default()
}
return l
}
29 changes: 29 additions & 0 deletions internal/loghelper/loghelper_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package loghelper

import (
"log/slog"
"testing"
)

func TestOrDiscard(t *testing.T) {
if got := OrDiscard(nil); got == nil {
t.Fatal("OrDiscard(nil) returned nil")
}
OrDiscard(nil).Info("must not panic")

l := slog.New(slog.DiscardHandler)
if got := OrDiscard(l); got != l {
t.Errorf("OrDiscard(l) = %p, want identity %p", got, l)
}
}

func TestOrDefault(t *testing.T) {
if got := OrDefault(nil); got != slog.Default() {
t.Errorf("OrDefault(nil) = %p, want slog.Default() %p", got, slog.Default())
}

l := slog.New(slog.DiscardHandler)
if got := OrDefault(l); got != l {
t.Errorf("OrDefault(l) = %p, want identity %p", got, l)
}
}
6 changes: 2 additions & 4 deletions pkg/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/radimsem/remindb/internal/fileext"
"github.com/radimsem/remindb/internal/ignore"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/internal/redaction"
"github.com/radimsem/remindb/internal/tempfile"
"github.com/radimsem/remindb/pkg/config"
Expand Down Expand Up @@ -131,10 +132,7 @@ func Compile(ctx context.Context, st *store.Store, opts ...Option) (*Result, err
opt(&o)
}

logger := o.logger
if logger == nil {
logger = slog.Default()
}
logger := loghelper.OrDefault(o.logger)

ctx, cancel := withTimeout(ctx, o.timeout)
defer cancel()
Expand Down
5 changes: 2 additions & 3 deletions pkg/mcp/initial.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ import (
"fmt"
"log/slog"

"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/compiler"
"github.com/radimsem/remindb/pkg/store"
)

// Run an initial compile when the store is empty; no-op otherwise.
func MaybeInitialCompile(ctx context.Context, st *store.Store, dir string, logger *slog.Logger) error {
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)

stats, err := st.GetStats(ctx)
if err != nil {
Expand Down
5 changes: 2 additions & 3 deletions pkg/mcp/ledger/ledger.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sync"

"github.com/radimsem/remindb/internal/contentid"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/config"
"github.com/radimsem/remindb/pkg/mcp/jsonlsink"
)
Expand Down Expand Up @@ -64,9 +65,7 @@ type Ledger struct {

// New opens (and compacts) the ledger under <workspace>/.remindb/sessions.
func New(workspace string, logger *slog.Logger) (*Ledger, error) {
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)
dir := filepath.Join(workspace, config.DirName, subDir)

// growth bounded by compact(), not rotation
Expand Down
5 changes: 2 additions & 3 deletions pkg/mcp/notify/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

mcp "github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/config"
"github.com/radimsem/remindb/pkg/mcp/resources"
)
Expand All @@ -31,9 +32,7 @@ type Publisher struct {
}

func NewPublisher(cfg config.ResourcesConfig, logger *slog.Logger) (*Publisher, error) {
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)

def := DefaultDebounce
if cfg.Debounce != nil {
Expand Down
5 changes: 2 additions & 3 deletions pkg/mcp/rescan.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/radimsem/remindb/internal/contentid"
"github.com/radimsem/remindb/internal/fileext"
"github.com/radimsem/remindb/internal/ignore"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/compiler"
"github.com/radimsem/remindb/pkg/config"
"github.com/radimsem/remindb/pkg/diff"
Expand Down Expand Up @@ -64,9 +65,7 @@ func NewRescanLoop(st *store.Store, dir string, interval time.Duration, cc confi
if interval <= 0 {
interval = defaultRescanInterval
}
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)
if status == nil {
status = rescanstat.New()
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/mcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/internal/redaction"
"github.com/radimsem/remindb/pkg/config"
"github.com/radimsem/remindb/pkg/logbuf"
Expand Down Expand Up @@ -102,10 +103,7 @@ func NewServer(st *store.Store, tracker *temperature.Tracker, cfg temperature.Co
opt(&o)
}

logger := o.logger
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger := loghelper.OrDiscard(o.logger)

transport := o.transport
if transport == "" {
Expand Down
5 changes: 2 additions & 3 deletions pkg/mcp/session/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

gomcp "github.com/modelcontextprotocol/go-sdk/mcp"
"github.com/radimsem/remindb/internal/contentid"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/mcp/ledger"
"github.com/radimsem/remindb/pkg/mcp/sessionlog"
)
Expand Down Expand Up @@ -59,9 +60,7 @@ type Registry struct {

// NewRegistry tracks live sessions; l (may be nil) persists their metrics.
func NewRegistry(srv *gomcp.Server, transport, listen string, l *ledger.Ledger, logger *slog.Logger) *Registry {
if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)

return &Registry{
srv: srv,
Expand Down
5 changes: 2 additions & 3 deletions pkg/temperature/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/radimsem/remindb/internal/contentid"
"github.com/radimsem/remindb/internal/loghelper"
"github.com/radimsem/remindb/pkg/config"
"github.com/radimsem/remindb/pkg/store"
)
Expand Down Expand Up @@ -49,9 +50,7 @@ func NewTracker(s NodeStore, dir string, bootstrap Config, logger *slog.Logger)
return nil, fmt.Errorf("invalid config: %w", err)
}

if logger == nil {
logger = slog.New(slog.DiscardHandler)
}
logger = loghelper.OrDiscard(logger)
return &Tracker{
store: s,
bootstrap: bootstrap,
Expand Down
Loading