From e32d4ede4431f0fc640b9b07aed4df1b69b96c11 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 10 May 2026 22:23:09 +0100 Subject: [PATCH] Expose engine narrative entry points for non-rendering callers The reporting engine's synthesis layer was reachable only through Generate/GenerateMulti, which always rendered PDF or CSV. Pulse Assistant needs the same retrospective synthesis (per-resource summary, fleet outliers, period comparison) in a form it can present in chat, not as a downloaded artifact. Add two non-rendering entry points to the Engine interface: NarrativeFor(req MetricReportRequest) (*Narrative, error) FleetNarrativeFor(req MultiReportRequest) (*FleetNarrative, error) Both run the same query path and the same narrator resolution as their rendering counterparts (heuristic by default, AI when the request supplies a narrator, fail-closed-to-heuristic on any narrator error) and return the structured narrative without invoking the fpdf/csv output stage. Test stubs in pkg/reporting and internal/api are updated to implement the extended interface. These are the seams the upcoming pulse_summarize Assistant tools wrap to answer questions like "what's hot on pve1 this week" or "where should I look across my fleet" without round-tripping through report generation. Same synthesis layer, no PDF involved. Also fixes a pre-existing flake in TestEngineGenerate_UsesSuppliedNarrator (metrics writes are async; the first Generate sometimes ran before the raw tier flushed). Wrapped in the same eventually-pattern used by the prior-period and findings-provider tests. --- internal/api/reporting_handlers_test.go | 16 +++ pkg/reporting/engine.go | 75 +++++++++++++ pkg/reporting/engine_narrative_test.go | 134 +++++++++++++++++++++++- pkg/reporting/reporting.go | 6 ++ pkg/reporting/reporting_test.go | 10 ++ 5 files changed, 239 insertions(+), 2 deletions(-) diff --git a/internal/api/reporting_handlers_test.go b/internal/api/reporting_handlers_test.go index 55ad5b355..b302cf39e 100644 --- a/internal/api/reporting_handlers_test.go +++ b/internal/api/reporting_handlers_test.go @@ -38,6 +38,22 @@ func (s *stubReportingEngine) GenerateMulti(req reporting.MultiReportRequest) ([ return s.data, s.contentType, nil } +func (s *stubReportingEngine) NarrativeFor(req reporting.MetricReportRequest) (*reporting.Narrative, error) { + s.lastReq = req + if s.err != nil { + return nil, s.err + } + return &reporting.Narrative{Source: reporting.NarrativeSourceHeuristic}, nil +} + +func (s *stubReportingEngine) FleetNarrativeFor(req reporting.MultiReportRequest) (*reporting.FleetNarrative, error) { + s.lastMulti = req + if s.err != nil { + return nil, s.err + } + return &reporting.FleetNarrative{Source: reporting.NarrativeSourceHeuristic}, nil +} + func TestReportingHandlers_MethodNotAllowed(t *testing.T) { handler := NewReportingHandlers(nil, nil) req := httptest.NewRequest(http.MethodPost, "/api/reporting", nil) diff --git a/pkg/reporting/engine.go b/pkg/reporting/engine.go index 40ff71e57..ce4cce0a2 100644 --- a/pkg/reporting/engine.go +++ b/pkg/reporting/engine.go @@ -564,6 +564,81 @@ func (e *ReportEngine) attachFleetNarrative(multiData *MultiReportData, req Mult multiData.FleetNarrative = &out } +// NarrativeFor returns the structured narrative for a single-resource +// report request without rendering PDF or CSV. This is the entry point +// non-rendering callers (e.g. the Assistant chat session) use when they +// want the same retrospective synthesis the report PDF carries but in a +// form they can present in chat. Same query path, same narrator +// resolution, same fail-closed-to-heuristic fallback as Generate; just +// no fpdf/csv output stage. +func (e *ReportEngine) NarrativeFor(req MetricReportRequest) (*Narrative, error) { + if e.getMetricsStore() == nil { + return nil, fmt.Errorf("metrics store not initialized") + } + reportData, err := e.queryMetrics(req) + if err != nil { + return nil, fmt.Errorf("failed to query metrics: %w", err) + } + e.attachNarrative(reportData, req) + if reportData.Narrative == nil { + return nil, fmt.Errorf("narrative generation produced no result") + } + return reportData.Narrative, nil +} + +// FleetNarrativeFor returns the structured fleet narrative for a +// multi-resource report request without rendering. Same shape as +// NarrativeFor for the single-resource path. Returns an error if no +// resource queries succeed; otherwise returns the narrative even if a +// subset of resources failed (the narrator is given what loaded, and +// the failed-resource list is logged at warn level by queryMetrics). +func (e *ReportEngine) FleetNarrativeFor(req MultiReportRequest) (*FleetNarrative, error) { + if e.getMetricsStore() == nil { + return nil, fmt.Errorf("metrics store not initialized") + } + multiData := &MultiReportData{ + Title: req.Title, + Start: req.Start, + End: req.End, + GeneratedAt: time.Now(), + } + if multiData.Title == "" { + multiData.Title = "Fleet Performance Report" + } + var successCount int + for _, resReq := range req.Resources { + resReq.Start = req.Start + resReq.End = req.End + resReq.MetricType = req.MetricType + if resReq.Narrator == nil { + resReq.Narrator = req.Narrator + } + if resReq.FindingsProvider == nil { + resReq.FindingsProvider = req.FindingsProvider + } + reportData, queryErr := e.queryMetrics(resReq) + if queryErr != nil { + log.Warn(). + Str("resourceType", resReq.ResourceType). + Str("resourceID", resReq.ResourceID). + Err(queryErr). + Msg("Skipping resource in fleet narrative: failed to query metrics") + continue + } + multiData.Resources = append(multiData.Resources, reportData) + multiData.TotalPoints += reportData.TotalPoints + successCount++ + } + if successCount == 0 { + return nil, fmt.Errorf("all resources failed to query metrics") + } + e.attachFleetNarrative(multiData, req) + if multiData.FleetNarrative == nil { + return nil, fmt.Errorf("fleet narrative generation produced no result") + } + return multiData.FleetNarrative, nil +} + // GetResourceTypeDisplayName returns a human-readable name for resource types. func GetResourceTypeDisplayName(resourceType string) string { switch CanonicalResourceType(resourceType) { diff --git a/pkg/reporting/engine_narrative_test.go b/pkg/reporting/engine_narrative_test.go index 63fb3f976..d3c81e56b 100644 --- a/pkg/reporting/engine_narrative_test.go +++ b/pkg/reporting/engine_narrative_test.go @@ -91,8 +91,18 @@ func TestEngineGenerate_UsesSuppliedNarrator(t *testing.T) { Format: FormatPDF, Narrator: stub, } - if _, _, err := engine.Generate(req); err != nil { - t.Fatalf("Generate: %v", err) + // Metrics writes are buffered; retry until the narrator sees stats. + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + stub.seen = NarrativeInput{} + stub.called = false + if _, _, err := engine.Generate(req); err != nil { + t.Fatalf("Generate: %v", err) + } + if _, ok := stub.seen.MetricStats["cpu"]; ok { + break + } + time.Sleep(50 * time.Millisecond) } if !stub.called { t.Fatal("narrator was not invoked") @@ -239,6 +249,126 @@ func TestEngineGenerate_FindingsProviderInvoked(t *testing.T) { } } +// TestEngineNarrativeFor_ReturnsStructuredNarrativeWithoutRendering +// verifies the non-rendering entry point used by Pulse Assistant tools: +// it must produce a Narrative grounded in the queried metrics without +// running the PDF or CSV generator. +func TestEngineNarrativeFor_ReturnsStructuredNarrativeWithoutRendering(t *testing.T) { + store := newReportingMetricsStore(t) + defer store.Close() + + now := time.Now() + nodeID := "node-narrate-1" + for i := 0; i < 12; i++ { + ts := now.Add(time.Duration(-60+i*5) * time.Minute) + store.Write("node", nodeID, "cpu", 55.0, ts) + } + store.Flush() + + engine := NewReportEngine(EngineConfig{MetricsStore: store}) + req := MetricReportRequest{ + ResourceType: "node", + ResourceID: nodeID, + Start: now.Add(-2 * time.Hour), + End: now.Add(time.Minute), + } + deadline := time.Now().Add(2 * time.Second) + var narrative *Narrative + for time.Now().Before(deadline) { + var err error + narrative, err = engine.NarrativeFor(req) + if err != nil { + t.Fatalf("NarrativeFor: %v", err) + } + if narrative != nil && len(narrative.Observations) > 0 { + break + } + time.Sleep(50 * time.Millisecond) + } + if narrative == nil { + t.Fatal("expected non-nil narrative") + } + if narrative.Source != NarrativeSourceHeuristic { + t.Errorf("Source = %q, want heuristic (no narrator supplied)", narrative.Source) + } + if len(narrative.Observations) == 0 { + t.Error("expected at least one observation from the heuristic narrator") + } +} + +// TestEngineFleetNarrativeFor_ReturnsStructuredFleetNarrativeWithoutRendering +// is the multi-resource counterpart. +func TestEngineFleetNarrativeFor_ReturnsStructuredFleetNarrativeWithoutRendering(t *testing.T) { + store := newReportingMetricsStore(t) + defer store.Close() + + now := time.Now() + for _, nodeID := range []string{"node-fleet-a", "node-fleet-b"} { + for i := 0; i < 6; i++ { + ts := now.Add(time.Duration(-30+i*5) * time.Minute) + store.Write("node", nodeID, "cpu", 60.0, ts) + } + } + store.Flush() + + engine := NewReportEngine(EngineConfig{MetricsStore: store}) + req := MultiReportRequest{ + Title: "Fleet narrative test", + Start: now.Add(-1 * time.Hour), + End: now.Add(time.Minute), + Resources: []MetricReportRequest{ + {ResourceType: "node", ResourceID: "node-fleet-a"}, + {ResourceType: "node", ResourceID: "node-fleet-b"}, + }, + } + deadline := time.Now().Add(2 * time.Second) + var fleet *FleetNarrative + for time.Now().Before(deadline) { + var err error + fleet, err = engine.FleetNarrativeFor(req) + if err != nil { + t.Fatalf("FleetNarrativeFor: %v", err) + } + if fleet != nil && fleet.HealthStatus != "" { + break + } + time.Sleep(50 * time.Millisecond) + } + if fleet == nil { + t.Fatal("expected non-nil fleet narrative") + } + if fleet.Source != NarrativeSourceHeuristic { + t.Errorf("Source = %q, want heuristic", fleet.Source) + } +} + +// TestEngineFleetNarrativeFor_NoResourcesReturnsError verifies the +// non-rendering fleet entry point matches GenerateMulti's error contract +// when zero resources are requested. +func TestEngineFleetNarrativeFor_NoResourcesReturnsError(t *testing.T) { + store := newReportingMetricsStore(t) + defer store.Close() + engine := NewReportEngine(EngineConfig{MetricsStore: store}) + now := time.Now() + req := MultiReportRequest{ + Start: now.Add(-1 * time.Hour), + End: now, + Resources: nil, + } + if _, err := engine.FleetNarrativeFor(req); err == nil { + t.Fatal("expected error when no resources are requested") + } +} + +// TestEngineFleetNarrativeFor_NoMetricsStoreReturnsError covers the +// guard at the top of the entry point. +func TestEngineFleetNarrativeFor_NoMetricsStoreReturnsError(t *testing.T) { + engine := NewReportEngine(EngineConfig{}) + if _, err := engine.FleetNarrativeFor(MultiReportRequest{}); err == nil { + t.Fatal("expected error when metrics store is nil") + } +} + type capturingNarrator struct { out Narrative err error diff --git a/pkg/reporting/reporting.go b/pkg/reporting/reporting.go index 7cb8da77d..ada7360f3 100644 --- a/pkg/reporting/reporting.go +++ b/pkg/reporting/reporting.go @@ -152,9 +152,15 @@ type MultiReportData struct { // Engine defines the interface for report generation. // This allows the enterprise version to provide PDF/CSV generation. +// +// NarrativeFor and FleetNarrativeFor return the structured narrative +// without rendering, for callers that want the synthesis layer in a +// non-PDF form (Pulse Assistant tool calls, programmatic consumers). type Engine interface { Generate(req MetricReportRequest) (data []byte, contentType string, err error) GenerateMulti(req MultiReportRequest) (data []byte, contentType string, err error) + NarrativeFor(req MetricReportRequest) (*Narrative, error) + FleetNarrativeFor(req MultiReportRequest) (*FleetNarrative, error) } var ( diff --git a/pkg/reporting/reporting_test.go b/pkg/reporting/reporting_test.go index 7d91812d0..9a552bd91 100644 --- a/pkg/reporting/reporting_test.go +++ b/pkg/reporting/reporting_test.go @@ -16,6 +16,16 @@ func (f *fakeEngine) GenerateMulti(req MultiReportRequest) ([]byte, string, erro return []byte("ok"), "text/plain", nil } +func (f *fakeEngine) NarrativeFor(req MetricReportRequest) (*Narrative, error) { + f.called = true + return &Narrative{Source: NarrativeSourceHeuristic}, nil +} + +func (f *fakeEngine) FleetNarrativeFor(req MultiReportRequest) (*FleetNarrative, error) { + f.called = true + return &FleetNarrative{Source: NarrativeSourceHeuristic}, nil +} + func TestSetGetEngine(t *testing.T) { engine := &fakeEngine{} SetEngine(engine)