From a1c2eb4b20e6e1a9532f03b63f3da4275071a7ff Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 24 Oct 2026 13:45:29 -0400 Subject: [PATCH 1/4] Reduce repeated work in Activity layouts, team disbanding and store reopening --- CLAUDE.md | 11 ++ internal/e2e/activity_scale_e2e_test.go | 1 + internal/e2e/skills_efficiency_e2e_test.go | 192 +++++++++++++++++++++ internal/e2e/teams_scale_e2e_test.go | 53 +++--- internal/store/sessions.go | 17 +- internal/store/sessions_open_test.go | 134 ++++++++++++++ internal/teams/lifecycle.go | 4 +- internal/teams/lifecycle_scale_test.go | 42 +++++ internal/tui3/activity_scale_test.go | 25 ++- internal/tui3/tasksplace.go | 39 +++-- internal/tui3/tasksplace_test.go | 4 +- internal/tui3/tasksplacewords_test.go | 6 +- internal/tui3/taskstable.go | 2 +- internal/tui3/taskstable_test.go | 4 +- 14 files changed, 473 insertions(+), 61 deletions(-) create mode 100644 internal/e2e/skills_efficiency_e2e_test.go create mode 100644 internal/store/sessions_open_test.go create mode 100644 internal/teams/lifecycle_scale_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 18edb0e752..d03cbe91d9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,6 +38,17 @@ and obsolete assumptions in the entry; see [changelog rules](docs/rules/changelo ## Build and test +Apply [Matt Pocock's engineering skills](https://github.com/mattpocock/skills) +when writing code: diagnose unexplained failures before changing code, use TDD +for fixes and features at agreed observable interfaces, and review the final +diff against both repository standards and the request. Pass these practices +to coding delegates. Claude can load them with +`claude plugin install mattpocock-skills@claude-plugins-official`. + +Keep module interfaces small and concentrate related behavior behind them. +Prefer reusing an existing module over adding a pass-through abstraction. +Measure performance before and after; preserve behavior during cleanup. + Use `make build` for the runnable binary at `bin/codeaf`. Rebuild after merging. Do not overwrite a running executable in place; use the installation/update path. diff --git a/internal/e2e/activity_scale_e2e_test.go b/internal/e2e/activity_scale_e2e_test.go index 5e8237ed7d..b45ebc52f6 100644 --- a/internal/e2e/activity_scale_e2e_test.go +++ b/internal/e2e/activity_scale_e2e_test.go @@ -145,4 +145,5 @@ func TestActivityScaleTerminalLatency(t *testing.T) { } sort.Float64s(samples) t.Logf("chats=40 tasks=5120 setup=%s first_screen=%s hover_ms p50=%.3f p95=%.3f samples=%v model_calls=0", setup, opening, samples[len(samples)/2], samples[len(samples)-1], samples) + r.quit() } diff --git a/internal/e2e/skills_efficiency_e2e_test.go b/internal/e2e/skills_efficiency_e2e_test.go new file mode 100644 index 0000000000..05b45914ab --- /dev/null +++ b/internal/e2e/skills_efficiency_e2e_test.go @@ -0,0 +1,192 @@ +//go:build e2e + +package e2e + +import ( + "context" + "database/sql" + "fmt" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/store" + teamstore "github.com/Agent-Field/codeaf/internal/teams" +) + +// Navigation and startup measurements exclude provider work entirely. The real +// binary still uses its ordinary configured model roles and rejects chat calls. +func efficiencyWire(t *testing.T) (string, *atomic.Int64) { + t.Helper() + calls := &atomic.Int64{} + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if strings.HasSuffix(r.URL.Path, "/chat/completions") { + calls.Add(1) + http.Error(w, "no model work belongs in this measurement", 500) + return + } + fmt.Fprint(w, `{"data":[]}`) + })) + t.Cleanup(s.Close) + return s.URL, calls +} + +func efficiencyWait(t *testing.T, r *rig, start time.Time, text string) time.Duration { + t.Helper() + for time.Since(start) < 30*time.Second { + if strings.Contains(r.capture(), text) { + return time.Since(start) + } + time.Sleep(5 * time.Millisecond) + } + t.Fatalf("screen did not display %q:\n%s", text, r.capture()) + return 0 +} + +func TestTeamsDisbandScaleTerminalLatency(t *testing.T) { + for _, size := range []struct{ teams, members int }{{100, 100}, {200, 200}} { + t.Run(fmt.Sprintf("%dteams_%dmembers", size.teams, size.members), func(t *testing.T) { + if _, err := exec.LookPath("tmux"); err != nil { + t.Skip("tmux required") + } + began := time.Now() + home := newHome(t, map[string]any{"memory.enabled": false, "model_pool": "off"}) + ws := newWorkspace(t, "disband-scale", false) + seedTeamsScale(t, size.teams, size.members, home, ws) + setup := time.Since(began) + url, calls := efficiencyWire(t) + r := startWithEnv(t, []string{config.APIKeyEnv + "=mock-efficiency-key", "CODEAF_BASE_URL=" + url}, "disband_scale", home, ws, 160, 55, "chat", "--model", "deepseek/deepseek-v4.1-flash", "--one-model", "--no-host") + r.skipSetup(t) + r.lit("/teams") + r.keys("Enter") + screen := r.waitFor(30*time.Second, "Scale team 000") + selected := false + for y, line := range strings.Split(screen, "\n") { + if x := strings.Index(line, "Scale team 000"); y > 2 && x >= 0 && x < 30 { + r.mouseClick(x+2, y+1) + selected = true + break + } + } + if !selected { + t.Fatal("no team sidebar target") + } + r.waitFor(30*time.Second, "Disband") + r.keys("c") + r.waitFor(30*time.Second, "Disband these teams:", "Cancel") + r.keys("Down") + start := time.Now() + if out, err := exec.Command("tmux", "send-keys", "-t", r.name, "Enter").CombinedOutput(); err != nil { + t.Fatalf("confirm: %v %s", err, out) + } + elapsed := efficiencyWait(t, r, start, "Scale team 000 is disbanded; conversations and current work continue") + f, err := teamstore.Load(home) + if err != nil { + t.Fatal(err) + } + closed, _ := f.Team("000000000001") + other, _ := f.Team("000000000002") + if !closed.Closed() || other.Closed() || len(closed.Members) != size.members || len(other.Members) != size.members { + t.Fatal("disband lost history or changed an unrelated team") + } + if calls.Load() != 0 { + t.Fatalf("disband made %d model calls", calls.Load()) + } + t.Logf("teams=%d memberships=%d setup=%s confirm_to_saved_notice=%s model_calls=0", size.teams, size.teams*size.members, setup, elapsed) + r.quit() + }) + } +} + +func TestSettledStoreTerminalStartup(t *testing.T) { + if _, err := exec.LookPath("tmux"); err != nil { + t.Skip("tmux required") + } + began := time.Now() + home := newHome(t, map[string]any{"memory.enabled": false, "model_pool": "off"}) + ws := newWorkspace(t, "settled-store", false) + path := filepath.Join(home, "graph.db") + s, err := store.Open(path) + if err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = db.Close() }) + _, err = db.Exec(`WITH RECURSIVE fixture(i) AS (VALUES(1) UNION ALL SELECT i+1 FROM fixture WHERE i < 1000000) +INSERT INTO events(ts,node_id,kind,payload) +SELECT '2026-01-01T00:00:00Z','','message_posted',json_object('session_id','mock-'||(i%1000),'role','user','body','Mock message') FROM fixture`) + if err != nil { + t.Fatal(err) + } + if _, err := db.Exec(`INSERT INTO messages(seq,ts,session_id,role,body) +SELECT seq,ts,json_extract(payload,'$.session_id'),'user','Mock message' FROM events WHERE kind='message_posted'`); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + s, err = store.Open(path) + if err != nil { + t.Fatal(err) + } + rooms, err := s.Sessions() + if err != nil || len(rooms) != 1000 { + t.Fatalf("fixture has %d rooms: %v", len(rooms), err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + setup := time.Since(began) + url, calls := efficiencyWire(t) + var samples []float64 + for i := 0; i < 3; i++ { + name := fmt.Sprintf("settled_start_%d", i) + target := fmt.Sprintf("p%d-%s", os.Getpid(), name) + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + ready := make(chan time.Time, 1) + start := time.Now() + // Observe the real first interactive surface without the rig's coarser + // startup polling. Fixture creation and later setup dismissal are separate. + go func() { + for ctx.Err() == nil { + out, _ := exec.CommandContext(ctx, "tmux", "capture-pane", "-p", "-t", target).Output() + screen := string(out) + if strings.Contains(screen, "alt+e effort") || strings.Contains(screen, "esc skips setup") || strings.Contains(screen, "↑↓ moves") || strings.Contains(screen, "needs you") { + ready <- time.Now() + return + } + time.Sleep(5 * time.Millisecond) + } + }() + r := startWithEnv(t, []string{config.APIKeyEnv + "=mock-efficiency-key", "CODEAF_BASE_URL=" + url}, name, home, ws, 160, 55, "chat", "--model", "deepseek/deepseek-v4.1-flash", "--one-model", "--no-host") + select { + case shown := <-ready: + samples = append(samples, float64(shown.Sub(start))/float64(time.Millisecond)) + case <-ctx.Done(): + t.Fatalf("no observed interactive surface:\n%s", r.capture()) + } + cancel() + r.kill() + } + if calls.Load() != 0 { + t.Fatalf("startup made %d model calls", calls.Load()) + } + sort.Float64s(samples) + t.Logf("messages=1000000 sessions=1000 setup=%s launch_to_first_surface_ms median=%.3f samples=%v model_calls=0", setup, samples[len(samples)/2], samples) +} diff --git a/internal/e2e/teams_scale_e2e_test.go b/internal/e2e/teams_scale_e2e_test.go index 28fe0abfcc..693e10a626 100644 --- a/internal/e2e/teams_scale_e2e_test.go +++ b/internal/e2e/teams_scale_e2e_test.go @@ -36,30 +36,7 @@ func runTeamsScaleTerminal(t *testing.T, count, members int) { began := time.Now() home := newHome(t, map[string]any{"model_pool": "off", "memory.enabled": false}) ws := newWorkspace(t, "teams-scale", false) - var teams []teamstore.Team - for i := 0; i < count; i++ { - tm := teamstore.Team{ID: fmt.Sprintf("%012x", i+1), Name: fmt.Sprintf("Scale team %03d", i)} - for j := 0; j < members; j++ { - sid := fmt.Sprintf("%016x", 0x6000000000000000+i*members+j) - dir := filepath.Join(home, "v3", "projects", strings.ReplaceAll(ws, string(filepath.Separator), "-"), sid) - if err := os.MkdirAll(dir, 0o700); err != nil { - t.Fatal(err) - } - at := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) - writeJSON(t, filepath.Join(dir, "meta.json"), map[string]any{"id": sid, "title": fmt.Sprintf("Mock member %03d %03d", i, j), "workspace": ws, "created": at, "lastUserAt": at}) - file := filepath.Join(dir, "transcript.jsonl") - journal := fmt.Sprintf("{\"type\":\"session\",\"version\":1,\"id\":%q,\"cwd\":%q}\n{\"type\":\"message\",\"role\":\"user\",\"content\":\"Review this mock change\",\"timestamp\":\"2026-01-01T00:00:00Z\"}\n{\"type\":\"message\",\"role\":\"assistant\",\"content\":\"Mock change reviewed\",\"timestamp\":\"2026-01-01T00:00:01Z\"}\n", sid, ws) - if err := os.WriteFile(file, []byte(journal), 0o600); err != nil { - t.Fatal(err) - } - tm.Members = append(tm.Members, teamstore.Member{Key: file, File: file, Where: ws, Handle: fmt.Sprintf("mock-%03d-%03d", i, j), Word: "Mock member", Home: j != 0, HandleBy: teamstore.HandleByTyped}) - if j == 0 { - tm.Manager = file - } - } - teams = append(teams, tm) - } - writeJSON(t, teamstore.Path(home), teamstore.File{Version: teamstore.Version, Teams: teams}) + seedTeamsScale(t, count, members, home, ws) seed := time.Since(began) var chats atomic.Int64 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -129,3 +106,31 @@ func runTeamsScaleTerminal(t *testing.T, count, members int) { } t.Logf("teams=%d memberships=%d setup=%s first_screen=%s hover_ms p50=%.3f p95=%.3f max=%.3f samples=%v model_calls=0", count, count*members, seed, opening, samples[len(samples)/2], samples[len(samples)-1], samples[len(samples)-1], samples) } + +func seedTeamsScale(t *testing.T, count, members int, home, ws string) { + t.Helper() + var teams []teamstore.Team + for i := 0; i < count; i++ { + tm := teamstore.Team{ID: fmt.Sprintf("%012x", i+1), Name: fmt.Sprintf("Scale team %03d", i)} + for j := 0; j < members; j++ { + sid := fmt.Sprintf("%016x", 0x6000000000000000+i*members+j) + dir := filepath.Join(home, "v3", "projects", strings.ReplaceAll(ws, string(filepath.Separator), "-"), sid) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + at := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + writeJSON(t, filepath.Join(dir, "meta.json"), map[string]any{"id": sid, "title": fmt.Sprintf("Mock member %03d %03d", i, j), "workspace": ws, "created": at, "lastUserAt": at}) + file := filepath.Join(dir, "transcript.jsonl") + journal := fmt.Sprintf("{\"type\":\"session\",\"version\":1,\"id\":%q,\"cwd\":%q}\n{\"type\":\"message\",\"role\":\"user\",\"content\":\"Review this mock change\",\"timestamp\":\"2026-01-01T00:00:00Z\"}\n{\"type\":\"message\",\"role\":\"assistant\",\"content\":\"Mock change reviewed\",\"timestamp\":\"2026-01-01T00:00:01Z\"}\n", sid, ws) + if err := os.WriteFile(file, []byte(journal), 0o600); err != nil { + t.Fatal(err) + } + tm.Members = append(tm.Members, teamstore.Member{Key: file, File: file, Where: ws, Handle: fmt.Sprintf("mock-%03d-%03d", i, j), Word: "Mock member", Home: j != 0, HandleBy: teamstore.HandleByTyped}) + if j == 0 { + tm.Manager = file + } + } + teams = append(teams, tm) + } + writeJSON(t, teamstore.Path(home), teamstore.File{Version: teamstore.Version, Teams: teams}) +} diff --git a/internal/store/sessions.go b/internal/store/sessions.go index f406094152..7fcfc90ba0 100644 --- a/internal/store/sessions.go +++ b/internal/store/sessions.go @@ -503,19 +503,18 @@ func (s *Store) SessionLastNonUserMessageSeq(sessionID string) (int64, error) { // construction: the anti-join leaves it a no-op once every session has its row, // and it runs at open rather than on any read path. // -// Idempotent is not free, though. The insert groups every message in the store -// to decide it has nothing to do, and it decides that on every open but the -// first — so the ordinary case pays a full scan of the thread to write nothing -// at all. The probe below asks the same question with the same anti-join and -// stops at the first row that answers it, which on a settled store is no rows -// and an index seek. +// Probe distinct room ids through the covering message index before looking +// for a missing session. A settled store still reads that index, but it pays +// one session lookup per room rather than one per message. Keep this probe on +// every open: an older writer can introduce missing rows after migration. func backfillSessions(db *sql.DB) error { var pending bool if err := db.QueryRow(` SELECT EXISTS ( - SELECT 1 FROM messages m - WHERE m.session_id <> '' - AND NOT EXISTS (SELECT 1 FROM sessions s WHERE s.id = m.session_id) + SELECT 1 FROM ( + SELECT DISTINCT session_id FROM messages WHERE session_id <> '' + ) m + WHERE NOT EXISTS (SELECT 1 FROM sessions s WHERE s.id = m.session_id) )`).Scan(&pending); err != nil { return err } diff --git a/internal/store/sessions_open_test.go b/internal/store/sessions_open_test.go new file mode 100644 index 0000000000..e75f25c04e --- /dev/null +++ b/internal/store/sessions_open_test.go @@ -0,0 +1,134 @@ +package store + +import ( + "fmt" + "path/filepath" + "testing" +) + +// A settled database may later acquire missing session rows from an older +// writer. Opening it must still discover those rooms without touching rooms +// whose title, tags, surface and activity were already recorded. +func TestOpenBackfillsLaterLegacySessionsAndPreservesExistingRooms(t *testing.T) { + path := filepath.Join(t.TempDir(), "graph.db") + s, err := Open(path) + if err != nil { + t.Fatal(err) + } + existing, err := s.OpenSession("existing", "Mock named room", "mock-surface") + if err != nil { + t.Fatal(err) + } + existing, err = s.RenameSessionTagged(existing.ID, existing.Title, []string{"mock"}) + if err != nil { + t.Fatal(err) + } + ids := []string{"legacy", "with spaces", "odd-\xff"} + expected := map[string]Session{} + for _, id := range ids { + first, err := s.PostMessage(Message{SessionID: id, Role: RoleUser, Body: "Mock first message"}) + if err != nil { + t.Fatal(err) + } + last, err := s.PostMessage(Message{SessionID: id, Role: RoleAgent, Body: "Mock last message"}) + if err != nil { + t.Fatal(err) + } + expected[id] = Session{ID: id, Created: first.Time, LastActive: last.Time} + } + if _, err := s.PostMessage(Message{Role: RoleUser, Body: "Mock unhomed message"}); err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + // Reopen once while settled, so a cached migration-completed flag would + // wrongly suppress the missing rows introduced after this point. + s, err = Open(path) + if err != nil { + t.Fatal(err) + } + if _, err := s.db.Exec(`DELETE FROM sessions WHERE id <> ?`, existing.ID); err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + s, err = Open(path) + if err != nil { + t.Fatal(err) + } + defer s.Close() + rooms, err := s.Sessions() + if err != nil { + t.Fatal(err) + } + if len(rooms) != len(expected)+1 { + t.Fatalf("got %d rooms, want %d", len(rooms), len(expected)+1) + } + for _, room := range rooms { + want := existing + if room.ID != existing.ID { + var ok bool + want, ok = expected[room.ID] + if !ok { + t.Fatalf("unexpected room %q", room.ID) + } + } + if !sameSession(room, want) { + t.Fatalf("room %q = %+v, want %+v", room.ID, room, want) + } + } +} + +// BenchmarkOpenSettledSessions includes the whole exported Open/Close path. +// Fixture creation and the first legacy migration are outside the timer. +func BenchmarkOpenSettledSessions(b *testing.B) { + for _, count := range []int{100000, 1000000} { + b.Run(fmt.Sprint(count), func(b *testing.B) { + path := filepath.Join(b.TempDir(), "graph.db") + s, err := Open(path) + if err != nil { + b.Fatal(err) + } + // Bulk fixture writes keep setup affordable. Events and messages have + // matching payloads; no personal database or provider is involved. + _, err = s.db.Exec(`WITH RECURSIVE fixture(i) AS (VALUES(1) UNION ALL SELECT i+1 FROM fixture WHERE i < ?) + INSERT INTO events(ts,node_id,kind,payload) + SELECT '2026-01-01T00:00:00Z','','message_posted',json_object('session_id','mock-'||(i%1000),'role','user','body','Mock message') FROM fixture`, count) + if err != nil { + b.Fatal(err) + } + _, err = s.db.Exec(`INSERT INTO messages(seq,ts,session_id,role,body) + SELECT seq,ts,json_extract(payload,'$.session_id'),'user','Mock message' FROM events WHERE kind='message_posted'`) + if err != nil { + b.Fatal(err) + } + if err := s.Close(); err != nil { + b.Fatal(err) + } + s, err = Open(path) + if err != nil { + b.Fatal(err) + } + rooms, err := s.Sessions() + if err != nil || len(rooms) != 1000 { + b.Fatalf("settled fixture: %d rooms, %v", len(rooms), err) + } + if err := s.Close(); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + s, err := Open(path) + if err != nil { + b.Fatal(err) + } + if err := s.Close(); err != nil { + b.Fatal(err) + } + } + }) + } +} diff --git a/internal/teams/lifecycle.go b/internal/teams/lifecycle.go index d193b84c55..f426a910c5 100644 --- a/internal/teams/lifecycle.go +++ b/internal/teams/lifecycle.go @@ -72,9 +72,11 @@ func (f *File) Disband(id string, at time.Time, report string) error { ids[d.ID] = true } independent := map[string]bool{} + // Read reporting before closing any team, once for the whole roster. + homes := f.Homes() for _, t := range f.Teams { for _, m := range t.Members { - if home, ok := f.Home(m.Key); ok && (ids[home.Via] || ids[home.Team]) { + if home, ok := homes[m.Key]; m.Key != "" && ok && (ids[home.Via] || ids[home.Team]) { independent[m.Key] = true } } diff --git a/internal/teams/lifecycle_scale_test.go b/internal/teams/lifecycle_scale_test.go new file mode 100644 index 0000000000..2462bc27c9 --- /dev/null +++ b/internal/teams/lifecycle_scale_test.go @@ -0,0 +1,42 @@ +package teams + +import ( + "fmt" + "testing" + "time" +) + +// BenchmarkDisbandScale measures the public mutation after fixture construction. +// Every conversation has an existing home, including the unrelated teams whose +// reporting must survive closing one branch. +func BenchmarkDisbandScale(b *testing.B) { + for _, size := range []struct{ teams, members int }{{100, 100}, {200, 200}} { + b.Run(fmt.Sprintf("%dteams_%dmembers", size.teams, size.members), func(b *testing.B) { + fixture := make([]Team, size.teams) + for i := range fixture { + id := fmt.Sprintf("%012x", i+1) + manager := fmt.Sprintf("manager-%d", i) + fixture[i] = Team{ID: id, Name: id, Manager: manager} + for j := 0; j < size.members; j++ { + fixture[i].Members = append(fixture[i].Members, Member{ + Key: fmt.Sprintf("worker-%d-%d", i, j), Home: true, + }) + } + } + at := time.Unix(100, 0) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + b.StopTimer() + f := &File{Teams: make([]Team, len(fixture))} + for j, team := range fixture { + f.Teams[j] = team.Clone() + } + b.StartTimer() + if err := f.Disband(fixture[0].ID, at, "finished"); err != nil { + b.Fatal(err) + } + } + }) + } +} diff --git a/internal/tui3/activity_scale_test.go b/internal/tui3/activity_scale_test.go index 2f37fdb465..42e56a27db 100644 --- a/internal/tui3/activity_scale_test.go +++ b/internal/tui3/activity_scale_test.go @@ -4,6 +4,7 @@ import ( "fmt" "math/rand" "reflect" + "strings" "testing" "time" @@ -108,13 +109,35 @@ func TestActivityParentIndexPreservesMalformedGraphPolicy(t *testing.T) { } } -func BenchmarkActivityScaleFrame(b *testing.B) { +// Repainting a large history must spend memory on layout facts, not another +// complete copy of every task and conversation record on the page. +func TestActivityScalePaintAllocationBudget(t *testing.T) { + a := activityScaleApp() + a.taskSheetBody(160, 45) + const budget = 4 << 20 + least := allocatedBytes(func() { + rows := a.taskSheetBody(160, 45) + if len(rows) == 0 || !strings.Contains(rows[0].text, "chats") { + t.Fatal("large history did not paint its conversation count") + } + }) + if least > budget { + t.Fatalf("Activity paint allocated %d bytes for 5,120 tasks; budget is %d", least, budget) + } +} + +func activityScaleApp() *app { world, now := activityScaleWorld(40, 128) a := &app{pal: newPalette(tokens.NoColor, false), width: 160, height: 55, clock: func() time.Time { return now }} a.raisePlace(pageTasks) a.taskSheet.world = world a.taskSheet.reading = readTasks(world, tasksMine{}, session.LastDays(now, 14), tasksSort{}, time.Time{}, now) a.taskSheet.awayAt = a.elsewhere().Read + return a +} + +func BenchmarkActivityScaleFrame(b *testing.B) { + a := activityScaleApp() a.taskSheetBody(160, 45) b.ReportAllocs() b.ResetTimer() diff --git a/internal/tui3/tasksplace.go b/internal/tui3/tasksplace.go index 051a6abcb7..4fd31d6119 100644 --- a/internal/tui3/tasksplace.go +++ b/internal/tui3/tasksplace.go @@ -588,11 +588,13 @@ type tasksLine struct { kind tasksLineKind // text is the words on a prose line. text string + // Payloads refer to immutable tree slots so each layout line keeps only its + // own geometry instead of copying both complete record types. // item is the work a task line and its tail are about. - item tasksItem - // chat is the conversation a [tasksLineChat] is about, and the zero value + item *tasksItem + // chat is the conversation a [tasksLineChat] is about, and nil // everywhere else. - chat tasksChat + chat *tasksChat // under says THE CONVERSATION THIS ROW CAME OUT OF IS ALREADY NAMED ABOVE IT, // by the chat row it hangs under or by the piece of work it was cut out of. // The row then spends those cells on its own name instead ([tasksChatRow]). @@ -704,8 +706,8 @@ func (r tasksReading) layout(width int) []tasksLine { // lines under it carry the rule of every ancestor that still has a sibling // to come, so a family's line is one unbroken stroke from its first row to // its last, however many live lines and grandchildren stand between them. - var work func(item tasksItem, depth int, last, named, nested bool, rails, branches []bool) - work = func(item tasksItem, depth int, last, named, nested bool, rails, branches []bool) { + var work func(item *tasksItem, depth int, last, named, nested bool, rails, branches []bool) + work = func(item *tasksItem, depth int, last, named, nested bool, rails, branches []bool) { key := tasksKeyOf(item.entry) kids := tree.kids[key] own := len(lines) @@ -740,7 +742,7 @@ func (r tasksReading) layout(width int) []tasksLine { lines = append(lines, line) // phone lane: a row becomes a two-line card a thumb goes into // (taskphone.go), and the second line belongs to the first. - if phone && tasksCardTail(item, r.now) != "" { + if phone && tasksCardTail(*item, r.now) != "" { lines = append(lines, tasksLine{ kind: tasksLineTail, item: item, owner: own, kin: line.underKin, @@ -764,8 +766,8 @@ func (r tasksReading) layout(width int) []tasksLine { return } below := append(append([]bool(nil), rails...), nested && !last) - for at, kid := range kids { - work(kid, depth+1, at == len(kids)-1, named, true, below, append(append([]bool(nil), branches...), !last)) + for at := range kids { + work(&kids[at], depth+1, at == len(kids)-1, named, true, below, append(append([]bool(nil), branches...), !last)) } } @@ -788,8 +790,9 @@ func (r tasksReading) layout(width int) []tasksLine { depth := 0 named := g.named if named { + chat := g.chat line := tasksLine{ - kind: tasksLineChat, chat: g.chat, owner: len(lines), + kind: tasksLineChat, chat: &chat, owner: len(lines), folds: len(g.roots) > 0, family: g.chat.key, kids: g.chat.kids, open: r.opens(g.chat.key), rank: g.chat.rank, } @@ -804,8 +807,8 @@ func (r tasksReading) layout(width int) []tasksLine { } depth = 1 } - for at, root := range g.roots { - work(root, depth, at == len(g.roots)-1, named, false, make([]bool, depth), nil) + for at := range g.roots { + work(&g.roots[at], depth, at == len(g.roots)-1, named, false, make([]bool, depth), nil) } } } @@ -1652,7 +1655,7 @@ func (r tasksReading) paint(lines []tasksLine, i, width int, pal palette, lit bo if tail < 1 { tail = 1 } - return lead + pal.dim(kin) + indent + placeFactInk(lit, pal)(fit(tasksCardTail(line.item, r.now), tail)) + return lead + pal.dim(kin) + indent + placeFactInk(lit, pal)(fit(tasksCardTail(*line.item, r.now), tail)) case tasksLinePlanUnder: // THE LIVE STEP LINE AND THE FIGURES UNDER IT, hung where the phone // card's second line hangs so a plan row's block reads at the same @@ -1663,7 +1666,7 @@ func (r tasksReading) paint(lines []tasksLine, i, width int, pal palette, lit bo if under < 1 { under = 1 } - rows := planUnderRows(line.item, under, pal) + rows := planUnderRows(*line.item, under, pal) if line.planUnder >= len(rows) { return "" } @@ -1679,7 +1682,7 @@ func (r tasksReading) paint(lines []tasksLine, i, width int, pal palette, lit bo if card < 1 { card = 1 } - name := tasksCardHead(line.item, card, pal, lit) + name := tasksCardHead(*line.item, card, pal, lit) fold := tasksFoldMark(line, pal) return lead + pal.dim(kin) + name + " " + pal.dim(fold) + pad(card-ansi.StringWidth(name)) + pad(1-ansi.StringWidth(fold)) + pad(tasksColumnAir) } @@ -1706,7 +1709,7 @@ func (r tasksReading) at(lines []tasksLine, i int) (tasksItem, bool) { if !lines[i].item.pick() { return tasksItem{}, false } - return lines[i].item, true + return *lines[i].item, true } // chatAt is the CONVERSATION drawn on one painted line, and whether the cursor @@ -1720,7 +1723,7 @@ func (r tasksReading) chatAt(lines []tasksLine, i int) (tasksChat, bool) { if i < 0 || i >= len(lines) || lines[i].kind != tasksLineChat { return tasksChat{}, false } - return lines[i].chat, true + return *lines[i].chat, true } // picks is whether the cursor may stand on one painted line AT ALL — a row of @@ -1981,7 +1984,7 @@ func tasksLabel(entry session.TaskIndexEntry) string { // // Project identity has its own column, leaving the title identical to Home. func tasksChatRow(line tasksLine, width int, now time.Time, folder, tilde string, by tasksSort, pal palette, lit bool) string { - chat := line.chat + chat := *line.chat name := chat.title project := strings.TrimSpace(chat.row.Project) if project == "" { @@ -2011,7 +2014,7 @@ func tasksChatRow(line tasksLine, width int, now time.Time, folder, tilde string // [tasksCardHead]'s ([tasksReading.paint] routes to it before this is called), // so the tier question is settled before we are here. func tasksRow(line tasksLine, width int, now time.Time, by tasksSort, pal palette, lit bool) string { - item := line.item + item := *line.item glyph, glyphInk := tasksGlyph(item, pal) lead := tasksBareLead + pal.dim(tasksTreeLead(line, width, pal)) + glyphInk(glyph) + " " cells := ansi.StringWidth(lead) diff --git a/internal/tui3/tasksplace_test.go b/internal/tui3/tasksplace_test.go index 59b794e740..1728a844f7 100644 --- a/internal/tui3/tasksplace_test.go +++ b/internal/tui3/tasksplace_test.go @@ -523,7 +523,7 @@ func TestARunningParentKeepsItsRefusedChildUnderIt(t *testing.T) { // [session.TaskStatus.RowWord] — the join that says the word once and // drops a reason that merely restates it. want := line.item.status().RowWord() - if got := tasksMiddle(line.item); got != want { + if got := tasksMiddle(*line.item); got != want { t.Fatalf("refused child reason = %q, want %q", got, want) } } @@ -658,7 +658,7 @@ func TestTheTaskNameIsWholeBeforeAnyFactGetsACell(t *testing.T) { // row, because a row that has already spent the one thing it was drawn to // say may not spend cells on a figure as well. item := reading.items[0] - narrow := plain(tasksRow(tasksLine{kind: tasksLineTask, item: item}, 30, now, tasksSort{}, newPalette(tokens.NoColor, false), false)) + narrow := plain(tasksRow(tasksLine{kind: tasksLineTask, item: &item}, 30, now, tasksSort{}, newPalette(tokens.NoColor, false), false)) if strings.Contains(narrow, "$") || strings.Contains(narrow, rowSep) { t.Fatalf("a frame too narrow for the name alone drew\n %s\nwant the cut name and no facts beside it", narrow) } diff --git a/internal/tui3/tasksplacewords_test.go b/internal/tui3/tasksplacewords_test.go index 2423283637..2424d010db 100644 --- a/internal/tui3/tasksplacewords_test.go +++ b/internal/tui3/tasksplacewords_test.go @@ -41,7 +41,7 @@ func TestARowSaysItsStateWordOnce(t *testing.T) { forbids: taskRecordStoppedWord + rowSep + taskRecordStoppedWord, }} { item := tasksItem{entry: tc.entry, runs: tc.runs} - row := plain(tasksRow(tasksLine{kind: tasksLineTask, item: item}, 120, now, tasksSort{}, newPalette(tokens.NoColor, false), false)) + row := plain(tasksRow(tasksLine{kind: tasksLineTask, item: &item}, 120, now, tasksSort{}, newPalette(tokens.NoColor, false), false)) word := taskStateWord(tc.entry, tc.runs) if strings.Contains(row, tc.forbids) { t.Fatalf("%s draws\n %s\nand says %q twice", tc.why, row, word) @@ -75,7 +75,7 @@ func TestConversationProjectColumnIncludesCurrentAndHomeProjects(t *testing.T) { now := time.Date(2026, time.September, 11, 9, 0, 0, 0, time.UTC) pal := newPalette(tokens.NoColor, false) draw := func(chat tasksChat, folder string) string { - line := tasksLine{kind: tasksLineChat, chat: chat} + line := tasksLine{kind: tasksLineChat, chat: &chat} return plain(tasksChatRow(line, 100, now, folder, "/home/pat", tasksSort{}, pal, false)) } root := func(title, project, dir string) tasksChat { @@ -173,7 +173,7 @@ func TestTheStateWordIsTheLastThingARowGivesUp(t *testing.T) { // and the reason under it ([tasksReasonLine]) — one row said whole rather // than twenty rows each missing the same word. for _, width := range []int{120, 100, tasksStateFloor} { - row := plain(tasksRow(tasksLine{kind: tasksLineTask, item: tc.item}, width, now, tasksSort{}, pal, false)) + row := plain(tasksRow(tasksLine{kind: tasksLineTask, item: &tc.item}, width, now, tasksSort{}, pal, false)) if !strings.Contains(row, word) { t.Fatalf("at %d columns %s reads\n %s\nand has given up %q, which is the one thing the list is read for", width, tc.why, strings.TrimRight(row, " "), word) diff --git a/internal/tui3/taskstable.go b/internal/tui3/taskstable.go index 3dde9bffcf..1d07c957b5 100644 --- a/internal/tui3/taskstable.go +++ b/internal/tui3/taskstable.go @@ -103,7 +103,7 @@ func tasksAgeHeaderHit(x, width int) bool { // ([tasksReasonLine]). It was on the row for as long as this page has existed and // it is what made the right edge prose. func tasksStateField(line tasksLine) rowField { - item := line.item + item := *line.item if note := tasksNote(item); note != "" { // THE WORDS FIRST AND THE WINDOW'S NAME AFTER THEM. `another window · // Fix the nil-map crash` is thirty-eight cells and this column has twenty, diff --git a/internal/tui3/taskstable_test.go b/internal/tui3/taskstable_test.go index 2bb66806b8..dc7bac5bad 100644 --- a/internal/tui3/taskstable_test.go +++ b/internal/tui3/taskstable_test.go @@ -151,7 +151,7 @@ func TestEveryRowOfOneFramePutsItsColumnsInTheSameCells(t *testing.T) { case tasksLineTask: want = rowTail([]rowField{tasksStateField(line)}, stateCells) case tasksLineChat: - want = rowTail([]rowField{tasksChatStateField(line.chat)}, stateCells) + want = rowTail([]rowField{tasksChatStateField(*line.chat)}, stateCells) default: continue } @@ -189,7 +189,7 @@ func TestEveryRowOfWorkSaysWhatItIs(t *testing.T) { if line.kind != tasksLineChat { continue } - got := rowTail([]rowField{tasksChatStateField(line.chat)}, tasksStateCells) + got := rowTail([]rowField{tasksChatStateField(*line.chat)}, tasksStateCells) if strings.TrimSpace(got) == "" { t.Fatalf("the conversation %q drew nothing in the state column", line.chat.title) } From 52f4edfa4425ff22ba498541449b23222ec70787 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 24 Oct 2026 13:46:00 -0400 Subject: [PATCH 2/4] Benchmark disbanding with managers present in real rosters --- internal/teams/lifecycle_scale_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/teams/lifecycle_scale_test.go b/internal/teams/lifecycle_scale_test.go index 2462bc27c9..652959dfcc 100644 --- a/internal/teams/lifecycle_scale_test.go +++ b/internal/teams/lifecycle_scale_test.go @@ -15,11 +15,11 @@ func BenchmarkDisbandScale(b *testing.B) { fixture := make([]Team, size.teams) for i := range fixture { id := fmt.Sprintf("%012x", i+1) - manager := fmt.Sprintf("manager-%d", i) + manager := fmt.Sprintf("worker-%d-0", i) fixture[i] = Team{ID: id, Name: id, Manager: manager} for j := 0; j < size.members; j++ { fixture[i].Members = append(fixture[i].Members, Member{ - Key: fmt.Sprintf("worker-%d-%d", i, j), Home: true, + Key: fmt.Sprintf("worker-%d-%d", i, j), Home: j > 0, }) } } From 5ddda45a1ba72c4739354bb9eb1862f2871bec88 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 24 Oct 2026 13:46:00 -0400 Subject: [PATCH 3/4] Document efficiency measurements and Activity allocation budget --- PERF.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/PERF.md b/PERF.md index ba5a769613..c6ae165a6f 100644 --- a/PERF.md +++ b/PERF.md @@ -17,6 +17,27 @@ is diagnostic; regression gates count deterministic work, bytes, allocations, request fanout, retained data and cache reuse. Do not gate on wall-clock thresholds. Use scripted providers and explicit synchronization rather than real sleeps. +Activity layout lines share immutable task records instead of copying both task +and conversation payloads into every line. Conversation overlays keep their own +copy, so live titles do not mutate the held reading. Paint scratch still expires +before the next input. `TestActivityScalePaintAllocationBudget` exercises the +actual body with 5,120 tasks and enforces a 4 MiB allocation budget per paint; +`BenchmarkActivityScaleFrame` and `TestActivityScaleTerminalLatency` measure the +same workload at the body and real-terminal interfaces. + +Team disbanding reads reporting homes once through the existing batch reader +before changing memberships. `BenchmarkDisbandScale` covers 10,000 and 40,000 +memberships; the batch trades temporary indexed storage for repeated scans. +`TestTeamsDisbandScaleTerminalLatency` measures confirmation through the visible +saved result, including persistence and redraw work. + +Store reopening probes distinct nonempty session IDs through the existing +message index before checking for missing session rows. It still scans message +IDs, and still discovers older writers' missing rows on a later open; no cached +migration flag skips that check. `BenchmarkOpenSettledSessions` measures the +whole public open/close operation. `TestSettledStoreTerminalStartup` separates +fixture construction from the first interactive frame of the running terminal. + ## What the gates check `make test-quick` checks compilation, vet, formatting, packed manuals, change From 1e8caa2fe8c46bada200aee8f048ea9e2ea40ae6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sat, 24 Oct 2026 13:54:57 -0400 Subject: [PATCH 4/4] Document efficiency fixes and coding workflow for PR 1925 --- .../1925-efficient-history-team-store.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 docs/changes/unreleased/1925-efficient-history-team-store.md diff --git a/docs/changes/unreleased/1925-efficient-history-team-store.md b/docs/changes/unreleased/1925-efficient-history-team-store.md new file mode 100644 index 0000000000..7f6e70a629 --- /dev/null +++ b/docs/changes/unreleased/1925-efficient-history-team-store.md @@ -0,0 +1,15 @@ +--- +kind: fixed +title: Reduce repeated work in large histories and team changes +pr: 1925 +surface: [chat, engine, docs] +invalidates: + - "Activity layouts previously copied full task and conversation records into each line. Lines now share immutable task records, while live conversation overlays keep independent copies; the 5,120-task paint has a 4 MiB allocation regression budget." + - "Disbanding a team previously scanned all memberships again for every reporting-home lookup. It now reuses the existing batched home reading before changing coordination, preserving history and unrelated memberships." + - "Opening a settled store previously checked for a missing session once per message. It now checks distinct nonempty session IDs through the existing covering index; later legacy writers' missing rows are still discovered." +--- + +No dependency, schema, persistent cache, product setting, or public interface is +added. Team mutation trades transient indexed storage for fewer repeated scans; +store reopening still scans message IDs. Contributor guidance records the Matt +Pocock diagnosis, TDD, module-design, and independent review workflow.