From b74646f8fa68a989f10c5ab9e7bc0a7579c2f937 Mon Sep 17 00:00:00 2001
From: Thando Mini <thando.mini@sanlam.co.za>
Date: Thu, 11 Jun 2026 12:32:52 +0200
Subject: [PATCH] perf(engine): collapse buildTimeline N+1 to one List per
 event type
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ReportBuilder.buildTimeline iterated five story event types, and inside
each ran one rb.es.List call PER story id. For a 20-story requirement
that's 5 × 20 = 100 sequential queries — 100 JSONL file reads on the
default store, 100 SQLite transactions on the SQLiteStore. Report
generation on long runs got noticeably slow.

Switch to one List per event type with no StoryID filter, then screen
results against the existing storyIDSet in memory. 5 queries total
regardless of story count. The filter is cheap: storyIDSet is already
constructed three lines above for the lookup that names each story
title in describeStoryEvent.

New TestReportBuilder_Build_TimelineFiltersByStoryOwnership seeds a
STORY_MERGED for a story id outside req-001's set and asserts it does
NOT appear in the timeline — guards the in-memory filter from
regressing into "list everything, forget to filter."

Surfaced by the 2026-06-11 Go audit (GO-MED-9).
---
 internal/engine/report.go      | 28 ++++++++++++++++------------
 internal/engine/report_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/internal/engine/report.go b/internal/engine/report.go
index 1ab3c83..a4f9949 100644
--- a/internal/engine/report.go
+++ b/internal/engine/report.go
@@ -357,20 +357,24 @@ func (rb *ReportBuilder) buildTimeline(reqID string, stories []state.Story) []Ti
 		storyIDSet[s.ID] = s.Title
 	}
 
+	// Previously this loop issued one List per (event-type × story-id) pair
+	// — 5 × len(stories) sequential queries. On JSONL stores that's 5N file
+	// reads; on SQLite it's 5N transactions. Issue ONE List per event type
+	// (no StoryID filter), then filter against storyIDSet in memory: 5
+	// queries total regardless of story count.
 	for _, evtType := range storyEventTypes {
-		for storyID, storyTitle := range storyIDSet {
-			evts, _ := rb.es.List(state.EventFilter{
-				Type:    evtType,
-				StoryID: storyID,
-			})
-			for _, evt := range evts {
-				entries = append(entries, TimelineEntry{
-					Timestamp:   evt.Timestamp,
-					EventType:   string(evt.Type),
-					StoryID:     storyID,
-					Description: rb.describeStoryEvent(evt.Type, storyTitle),
-				})
+		evts, _ := rb.es.List(state.EventFilter{Type: evtType})
+		for _, evt := range evts {
+			storyTitle, owned := storyIDSet[evt.StoryID]
+			if !owned {
+				continue
 			}
+			entries = append(entries, TimelineEntry{
+				Timestamp:   evt.Timestamp,
+				EventType:   string(evt.Type),
+				StoryID:     evt.StoryID,
+				Description: rb.describeStoryEvent(evt.Type, storyTitle),
+			})
 		}
 	}
 
diff --git a/internal/engine/report_test.go b/internal/engine/report_test.go
index 6d23c30..878d476 100644
--- a/internal/engine/report_test.go
+++ b/internal/engine/report_test.go
@@ -145,6 +145,40 @@ func setupReportStores(t *testing.T) (state.EventStore, *state.SQLiteStore, func
 	return es, ps, cleanup
 }
 
+// TestReportBuilder_Build_TimelineFiltersByStoryOwnership guards the
+// post-N+1 path: buildTimeline issues one List per event type (no StoryID
+// filter) and screens via storyIDSet in memory. The regression to prevent
+// is "events from stories belonging to OTHER requirements leak into the
+// timeline." Seed a STORY_MERGED for a foreign story id, then assert it is
+// NOT in the report.
+func TestReportBuilder_Build_TimelineFiltersByStoryOwnership(t *testing.T) {
+	es, ps, cleanup := setupReportStores(t)
+	defer cleanup()
+
+	// Foreign story id — does NOT appear in stories(req-001).
+	foreignMerged := state.NewEvent(state.EventStoryMerged, "merger", "FOREIGN-STORY-Z", nil)
+	if err := es.Append(foreignMerged); err != nil {
+		t.Fatalf("append foreign event: %v", err)
+	}
+	if err := ps.Project(foreignMerged); err != nil {
+		t.Fatalf("project foreign event: %v", err)
+	}
+
+	cfg := config.DefaultConfig()
+	rb := engine.NewReportBuilder(es, ps, cfg)
+
+	report, err := rb.Build("req-001")
+	if err != nil {
+		t.Fatalf("Build: %v", err)
+	}
+
+	for _, entry := range report.Timeline {
+		if entry.StoryID == "FOREIGN-STORY-Z" {
+			t.Fatalf("foreign story %q leaked into timeline (in-memory filter regression)", entry.StoryID)
+		}
+	}
+}
+
 func TestReportBuilder_Build_BasicFields(t *testing.T) {
 	es, ps, cleanup := setupReportStores(t)
 	defer cleanup()