kg: review follow-ups — observation-age stats, stats/health parity, embedding scan, hub install reconciliation

src/kg/health.go

@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"sort"
+	"time"
 
 	"github.com/cortexa-llc/mcp/kg/internal/knowledge"
 	"github.com/spf13/cobra"
@@ -72,7 +73,7 @@ func runHealth(root, scopeName string, jsonOut bool, out io.Writer) error {
 	aiDir := filepath.Join(root, ".ai")
 	projectID := projectIDFromCwd(root)
 
-	dbPath, scopeName, err := resolveHealthDB(aiDir, scopeName)
+	dbPath, scopeName, err := resolveScopeDB(aiDir, scopeName)
 	if err != nil {
 		return err
 	}
@@ -121,15 +122,29 @@ func runHealth(root, scopeName string, jsonOut bool, out io.Writer) error {
 	return nil
 }
 
-// resolveHealthDB resolves the database path: explicit scope, else the
-// default scope, else the legacy knowledge.db. Returns the path and the scope
-// name actually used ("" for legacy). This mirrors `kg stats` with one
-// deliberate divergence: stats silently falls back to the legacy database
-// when a named scope cannot be loaded, health errors (see below).
-// A named scope that cannot be loaded is an error, never a silent fallback —
-// reporting the legacy database's health under a scope the user asked for
-// would be a wrong answer, not a degraded one.
-func resolveHealthDB(aiDir, scopeName string) (string, string, error) {
+// resolveScopeDB resolves the database path for the read-only report commands
+// (kg health, kg stats): the scope the user named, else the configured default
+// scope, else the legacy knowledge.db. Returns the path and the scope name
+// actually used ("" for legacy).
+//
+// The two sources of a scope name are held to different standards, which is
+// why requested is separate from the default rather than pre-resolved by the
+// caller:
+//
+//   - A scope the user NAMED that cannot be loaded is always an error.
+//     Reporting the legacy database under a scope someone asked for by name is
+//     a wrong answer, not a degraded one.
+//   - A default scope inherited from config.json falls back to the legacy
+//     database when the project has no scope configs at all. config.json
+//     travels with a repo and SetDefaultScope does not verify the scope
+//     exists, so a stale defaultScope naming nothing is a configuration
+//     leftover, not a request — failing there would break `kg stats` in
+//     repositories where it used to work.
+//
+// A default scope in a project that DOES have scope configs still errors: the
+// scopes are real, so a default naming a missing one is a genuine mistake.
+func resolveScopeDB(aiDir, requested string) (string, string, error) {
+	scopeName := requested
 	if scopeName == "" {
 		defaultScope, err := knowledge.GetDefaultScope(aiDir)
 		if err != nil {
@@ -141,6 +156,16 @@ func resolveHealthDB(aiDir, scopeName string) (string, string, error) {
 		return filepath.Join(aiDir, "knowledge.db"), "", nil
 	}
 
+	if requested == "" {
+		configs, err := knowledge.ListScopeConfigs(aiDir)
+		if err != nil {
+			return "", "", err
+		}
+		if len(configs) == 0 {
+			return filepath.Join(aiDir, "knowledge.db"), "", nil
+		}
+	}
+
 	cfg, err := knowledge.LoadScopeConfig(aiDir, scopeName)
 	if err != nil {
 		return "", "", fmt.Errorf("load scope %q: %w", scopeName, err)
@@ -220,6 +245,12 @@ func printHealth(out io.Writer, o healthOutput, snapPath string) {
 		fmt.Fprintf(out, "\nNo previous snapshot — growth will be reported from the next run.\n")
 	}
 
+	if oa := o.Current.ObservationAge; oa != nil {
+		gen := o.Current.GeneratedAt
+		fmt.Fprintf(out, "\nObservation age: newest %s, median %s, oldest %s\n",
+			humanAge(gen.Sub(oa.Newest)), humanAge(gen.Sub(oa.Median)), humanAge(gen.Sub(oa.Oldest)))
+	}
+
 	share := 0.0
 	if o.Current.Observations > 0 {
 		share = float64(o.Current.ZeroTimestampObservations) / float64(o.Current.Observations) * 100
@@ -232,6 +263,34 @@ func printHealth(out io.Writer, o healthOutput, snapPath string) {
 	fmt.Fprintf(out, "\nSnapshot written to %s\n", snapPath)
 }
 
+// humanAge renders a duration as a coarse age ("3h", "2d", "5mo") for the
+// observation-age line; precision beyond this is noise in a health report.
+func humanAge(d time.Duration) string {
+	const (
+		day   = 24 * time.Hour
+		month = 30 * day
+		year  = 12 * month // 360d, so the ladder never prints "12mo" before "1y"
+	)
+	switch {
+	case d < 0:
+		// A stored timestamp ahead of the report's own clock: skew between
+		// machines pushing to a shared hub graph, or a hand-set created_at.
+		return "in the future"
+	case d < time.Minute:
+		return "just now"
+	case d < time.Hour:
+		return fmt.Sprintf("%dm", int(d.Minutes()))
+	case d < day:
+		return fmt.Sprintf("%dh", int(d.Hours()))
+	case d < month:
+		return fmt.Sprintf("%dd", int(d/day))
+	case d < year:
+		return fmt.Sprintf("%dmo", int(d/month))
+	default:
+		return fmt.Sprintf("%dy", int(d/year))
+	}
+}
+
 // formatEntityTypes renders the by-type counts sorted by count descending,
 // then name, so the report is stable between runs.
 func formatEntityTypes(byType map[string]int) []string {

src/kg/health_test.go

@@ -3,18 +3,22 @@ package main
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/cortexa-llc/mcp/kg/internal/knowledge"
 )
 
 // seedHealthFixture creates <root>/.ai/knowledge.db with a small graph:
-// three entities (one orphaned), two observations (one [OBSOLETE-marked),
-// and one relation. The store is closed before returning so runHealth can
-// open the database read-only.
+// three entities (one orphaned), one relation, and six observations — two
+// written normally (one [OBSOLETE-marked), two aged into legacy state (NULL
+// and stored-zero created_at), and two back-dated to 2020 and 2022 to pin the
+// age stats. The store is closed before returning so runHealth can open the
+// database read-only.
 func seedHealthFixture(t *testing.T, root string) {
 	t.Helper()
 
@@ -52,17 +56,24 @@ func seedHealthFixture(t *testing.T, root string) {
 		t.Fatalf("CreateEntity: %v", err)
 	}
 
-	// Two observations with genuinely legacy STORED timestamps — one NULL, one
-	// stored zero — written the only way this writer can produce them: by
-	// mutating created_at after the fact. This is what the zero-timestamp
-	// metric exists to count; a fixture whose every row carries a real
-	// timestamp cannot fail a broken counter. The stored-zero row in
+	// Four observations with STORED timestamps this writer cannot produce
+	// directly — two legacy (NULL and stored zero), two back-dated — written
+	// the only way it can: by mutating created_at after the fact. The legacy
+	// pair is what the zero-timestamp metric exists to count; a fixture whose
+	// every row carries a real timestamp cannot fail a broken counter. The stored-zero row in
 	// particular guards against binding the zero time as a Go parameter,
 	// which go-kuzu mangles through UnixNano into a 1754 date that matches
 	// nothing.
 	for _, mutation := range []string{
 		"SET o.created_at = NULL",
 		`SET o.created_at = timestamp("0001-01-01 00:00:00")`,
+		// Two dated rows pin the age stats: with the two freshly-written
+		// observations above, the four timestamped rows sort as
+		// [2020, 2022, now, now], so oldest must be 2020 and the (lower)
+		// median must be 2022 — a median query that grabs the first, last,
+		// or a legacy row cannot pass.
+		`SET o.created_at = timestamp("2020-01-01 00:00:00")`,
+		`SET o.created_at = timestamp("2022-06-15 00:00:00")`,
 	} {
 		obs, err := store.CreateObservation(e2.ID, "legacy-era note", projectID)
 		if err != nil {
@@ -110,18 +121,31 @@ func TestHealthCommandReportsMetricsAndGrowth(t *testing.T) {
 	if out.Current.Relations != 1 {
 		t.Errorf("relations = %d, want 1", out.Current.Relations)
 	}
-	if out.Current.Observations != 4 {
-		t.Errorf("observations = %d, want 4", out.Current.Observations)
+	if out.Current.Observations != 6 {
+		t.Errorf("observations = %d, want 6", out.Current.Observations)
 	}
-	// Exactly the two deliberately aged rows — not 0 (metric dead) and not 4
-	// (metric counting scan artifacts instead of stored values).
+	// Exactly the two legacy rows — not 0 (metric dead) and not 6 (metric
+	// counting scan artifacts instead of stored values).
 	if out.Current.ZeroTimestampObservations != 2 {
 		t.Errorf("zero-timestamp observations = %d, want 2 (one NULL + one stored zero)",
 			out.Current.ZeroTimestampObservations)
 	}
 	if out.Current.OrphanedEntities != 1 {
 		t.Errorf("orphaned entities = %d, want 1", out.Current.OrphanedEntities)
 	}
+	if oa := out.Current.ObservationAge; oa == nil {
+		t.Error("observation age missing with 4 timestamped observations")
+	} else {
+		if oa.Oldest.Year() != 2020 {
+			t.Errorf("oldest observation year = %d, want 2020", oa.Oldest.Year())
+		}
+		if oa.Median.Year() != 2022 {
+			t.Errorf("median observation year = %d, want 2022 (lower median of [2020 2022 now now])", oa.Median.Year())
+		}
+		if time.Since(oa.Newest) > time.Minute {
+			t.Errorf("newest observation = %v, want within the last minute", oa.Newest)
+		}
+	}
 	if out.Current.ObsoleteObservations != 1 {
 		t.Errorf("[OBSOLETE observations = %d, want 1", out.Current.ObsoleteObservations)
 	}
@@ -202,9 +226,115 @@ func TestHealthCommandHumanOutput(t *testing.T) {
 	if err := runHealth(root, "", false, &buf); err != nil {
 		t.Fatalf("runHealth: %v", err)
 	}
-	for _, want := range []string{"legacy, age unknown", "Orphaned entities", "No previous snapshot"} {
+	for _, want := range []string{"legacy, age unknown", "Orphaned entities", "No previous snapshot", "Observation age: newest"} {
 		if !strings.Contains(buf.String(), want) {
 			t.Errorf("human output missing %q:\n%s", want, buf.String())
 		}
 	}
 }
+
+// An explicitly named scope that cannot be loaded is an error for both
+// report commands (kg health and kg stats share resolveScopeDB) — never a
+// silent fallback to the legacy database, which answers the wrong question.
+func TestResolveScopeDBErrorsOnUnloadableScope(t *testing.T) {
+	aiDir := filepath.Join(t.TempDir(), ".ai")
+	if err := os.MkdirAll(aiDir, 0o755); err != nil {
+		t.Fatalf("MkdirAll: %v", err)
+	}
+	if _, _, err := resolveScopeDB(aiDir, "no-such-scope"); err == nil {
+		t.Error("resolveScopeDB with an unloadable scope returned nil error, want failure")
+	}
+	// No scope named and none configured: the legacy database, no error.
+	dbPath, scopeName, err := resolveScopeDB(aiDir, "")
+	if err != nil || scopeName != "" || filepath.Base(dbPath) != "knowledge.db" {
+		t.Errorf("legacy resolution = (%q, %q, %v), want knowledge.db path, empty scope, nil", dbPath, scopeName, err)
+	}
+}
+
+// humanAge's thresholds, which the report's only age line depends on. The
+// human-output test can pass on an empty or misordered rendering, so the
+// boundaries are pinned here directly.
+func TestHumanAge(t *testing.T) {
+	cases := []struct {
+		d    time.Duration
+		want string
+	}{
+		{30 * time.Second, "just now"},
+		{time.Minute, "1m"},
+		{59 * time.Minute, "59m"},
+		{time.Hour, "1h"},
+		{23 * time.Hour, "23h"},
+		{24 * time.Hour, "1d"},
+		{29 * 24 * time.Hour, "29d"},
+		{30 * 24 * time.Hour, "1mo"},
+		// The month/year boundary is 12 months of 30 days, so the ladder goes
+		// 11mo -> 1y with no "12mo" step.
+		{359 * 24 * time.Hour, "11mo"},
+		{360 * 24 * time.Hour, "1y"},
+		{364 * 24 * time.Hour, "1y"},
+		{365 * 24 * time.Hour, "1y"},
+		{800 * 24 * time.Hour, "2y"},
+		// A stored timestamp ahead of the report's clock (machine skew, or a
+		// hand-set created_at) must be named, not rendered as "just now".
+		{-time.Hour, "in the future"},
+	}
+	for _, tc := range cases {
+		if got := humanAge(tc.d); got != tc.want {
+			t.Errorf("humanAge(%v) = %q, want %q", tc.d, got, tc.want)
+		}
+	}
+}
+
+// The age line names newest, median, and oldest in that order with real
+// values — an assertion on the label alone passes even if humanAge returns
+// empty strings or the three are rendered in the wrong order.
+func TestHealthHumanOutputAgeLine(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	root := t.TempDir()
+	seedHealthFixture(t, root)
+
+	var buf bytes.Buffer
+	if err := runHealth(root, "", false, &buf); err != nil {
+		t.Fatalf("runHealth: %v", err)
+	}
+	// The fixture's timestamped rows are [2020, 2022, now, now], so the line
+	// must read newest "just now", median ~4y, oldest ~6y — ordered oldest
+	// last. Years drift with the wall clock, so match the shape and the
+	// ordering rather than exact ages.
+	line := ""
+	for _, l := range strings.Split(buf.String(), "\n") {
+		if strings.HasPrefix(l, "Observation age:") {
+			line = l
+			break
+		}
+	}
+	if line == "" {
+		t.Fatalf("no observation-age line in report:\n%s", buf.String())
+	}
+	newestIdx := strings.Index(line, "newest just now")
+	medianIdx := strings.Index(line, "median ")
+	oldestIdx := strings.Index(line, "oldest ")
+	if newestIdx < 0 || medianIdx < 0 || oldestIdx < 0 {
+		t.Fatalf("age line missing a labelled value: %q", line)
+	}
+	if !(newestIdx < medianIdx && medianIdx < oldestIdx) {
+		t.Errorf("age line out of order (newest, median, oldest): %q", line)
+	}
+	if strings.Contains(line, "median y") || strings.Contains(line, "median ,") {
+		t.Errorf("age line has an empty median value: %q", line)
+	}
+	// Median (2022) must read as a smaller age than oldest (2020).
+	var medianYears, oldestYears int
+	if _, err := fmt.Sscanf(line[medianIdx:], "median %dy", &medianYears); err != nil {
+		t.Fatalf("median value is not a year age in %q: %v", line, err)
+	}
+	if _, err := fmt.Sscanf(line[oldestIdx:], "oldest %dy", &oldestYears); err != nil {
+		t.Fatalf("oldest value is not a year age in %q: %v", line, err)
+	}
+	if medianYears >= oldestYears {
+		t.Errorf("median age %dy is not younger than oldest %dy: %q", medianYears, oldestYears, line)
+	}
+}