Skip to content

Commit a5caeef

Browse files
cpcloudclaude
andcommitted
fix(llm): address PR review feedback
- Gate dashboard refresh key on insightsWanted() (checks both insightsEnabled and llmClient) instead of insightsEnabled alone - Validate context_length >= 0 during config load with clear error - Update DataDump() doc comment to reflect that entity IDs are now included Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cf639bd commit a5caeef

5 files changed

Lines changed: 33 additions & 3 deletions

File tree

internal/app/dashboard_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,20 @@ import (
1111

1212
"github.com/charmbracelet/lipgloss"
1313
"github.com/cpcloud/micasa/internal/data"
14+
"github.com/cpcloud/micasa/internal/llm"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
1617
)
1718

19+
// testDummyLLMClient returns a non-nil LLM client for tests that need
20+
// insightsWanted() to return true. Not connected to a real server.
21+
func testDummyLLMClient(t *testing.T) *llm.Client {
22+
t.Helper()
23+
c, err := llm.NewClient("llamacpp", "http://127.0.0.1:1", "test", "", time.Second, 0)
24+
require.NoError(t, err)
25+
return c
26+
}
27+
1828
// nonEmptyDashboard returns a minimal dashboardData that is not empty,
1929
// for tests that just need the dashboard overlay to render.
2030
func nonEmptyDashboard() dashboardData {
@@ -1415,6 +1425,7 @@ func TestInsightsRefreshKey_MarksStale(t *testing.T) {
14151425
t.Parallel()
14161426
m := newTestModel(t)
14171427
m.insightsEnabled = true
1428+
m.llmClient = testDummyLLMClient(t)
14181429
m.width = 120
14191430
m.height = 40
14201431

internal/app/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -679,7 +679,7 @@ func (m *Model) handleDashboardKeys(key tea.KeyMsg) (tea.Cmd, bool) {
679679
// Block column movement on dashboard.
680680
return nil, true
681681
case keyR:
682-
if m.insightsEnabled {
682+
if m.insightsWanted() {
683683
cmd := m.refreshInsights()
684684
m.setStatusInfo("refreshing insights")
685685
return cmd, true

internal/config/config.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,14 @@ func LoadFromPath(path string) (Config, error) {
504504
)
505505
}
506506

507+
// Validate context_length.
508+
if cfg.LLM.ContextLength < 0 {
509+
return cfg, fmt.Errorf(
510+
"llm.context_length: must be >= 0, got %d",
511+
cfg.LLM.ContextLength,
512+
)
513+
}
514+
507515
// Validate timeouts.
508516
if cfg.LLM.Timeout != "" {
509517
d, err := time.ParseDuration(cfg.LLM.Timeout)

internal/config/config_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,17 @@ context_length = 65536
861861
assert.Equal(t, 65536, cfg.LLM.ExtractionConfig().ContextLength)
862862
}
863863

864+
func TestContextLengthRejectsNegative(t *testing.T) {
865+
path := writeConfig(t, `[llm]
866+
provider = "ollama"
867+
model = "qwen3"
868+
context_length = -1
869+
`)
870+
_, err := LoadFromPath(path)
871+
require.Error(t, err)
872+
assert.Contains(t, err.Error(), "context_length")
873+
}
874+
864875
func TestExtractionConfigInheritsBase(t *testing.T) {
865876
path := writeConfig(t, `[llm]
866877
provider = "ollama"

internal/data/query.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,8 @@ func firstWord(s string) string {
261261
//
262262
// The output is optimized for small LLMs: null/empty values are omitted,
263263
// money columns (ending in "_ct") are formatted as dollars, and internal
264-
// columns (id, created_at, updated_at, deleted_at) are excluded to reduce
265-
// noise.
264+
// columns (created_at, updated_at, deleted_at) are excluded to reduce
265+
// noise. Entity IDs are included so the LLM can reference specific records.
266266
func (s *Store) DataDump() string {
267267
names, err := s.TableNames()
268268
if err != nil {

0 commit comments

Comments
 (0)