From ebbd30fe8af974f8ffae50f9c582ee128d7bcc08 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Thu, 13 Aug 2026 10:16:32 +0300 Subject: [PATCH] beads: report a clipped read from every projection --- beads/build.go | 45 ++++-- beads/milestones.go | 37 ++++- beads/model.go | 34 ++++- beads/truncation_test.go | 309 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 409 insertions(+), 16 deletions(-) create mode 100644 beads/truncation_test.go diff --git a/beads/build.go b/beads/build.go index e0fd6eed21bfc7fae4caf70812e90e239b39f425..51031d70561b845aae813eef56460e8d8ecd5062 100644 --- a/beads/build.go +++ b/beads/build.go @@ -26,9 +26,12 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values return nil, err } // Optional tables: absent ones degrade to empty rather than failing the view. - labels, _, _ := readRowsOptional(ctx, sess, ref, "labels") - statuses, _, _ := readRowsOptional(ctx, sess, ref, "custom_statuses") + labels, labelsTotal, _ := readRowsOptional(ctx, sess, ref, "labels") + statuses, statusesTotal, _ := readRowsOptional(ctx, sess, ref, "custom_statuses") + // The board's flag names the two required tables it buckets from, which is + // what it has always reported; widening it to the optional ones is a separate + // question from this one. truncated := issuesTotal > Max || depsTotal > Max shownOf := issuesTotal @@ -48,8 +51,16 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values // Detail mode: a named issue short-circuits the board build. if want := query.Get("issue"); want != "" { + // The detail pane draws on every table read above — its labels come from + // labels, its lane from custom_statuses — so its clip flag covers all of + // them, plus the two tables it reads itself. Each total is already in hand; + // none of this costs a second read. + clip := readClip{ + truncated: truncated || labelsTotal > Max || statusesTotal > Max, + issuesTotal: issuesTotal, + } return buildDetail(ctx, sess, ref, want, issues, issueCols, deps, depCols, - labelsByIssue, catByStatus, catByIssue), nil + labelsByIssue, catByStatus, catByIssue, clip), nil } // Board mode: parse the sticky filters and collect dropdown options from the @@ -146,6 +157,15 @@ func Build(ctx context.Context, sess BrowseSession, ref string, query url.Values return data, nil } +// readClip is what the row reads reported about their own completeness: whether +// any table came back clipped at Max, and the issues table's true total. It is +// threaded from Build into buildDetail so the detail pane can say its read was +// partial — every number in it comes from reads already made. +type readClip struct { + truncated bool // some table this projection reads exceeded Max + issuesTotal int // the issues table's reported total, clipped or not +} + // buildDetail assembles the single-issue view: the issue's own fields, its // dependency edges in both directions (target title/status resolved), its // comments thread, and a merged history timeline. When the issue is an epic @@ -157,6 +177,7 @@ func buildDetail( deps *browse.RowPage, depCols map[string]int, labelsByIssue map[string][]string, catByStatus, catByIssue map[string]string, + clip readClip, ) *Data { // id → (title, status, whole row) for edge labels and the subtask rollup. titleByIssue := map[string]string{} @@ -173,10 +194,12 @@ func buildDetail( } } - data := &Data{Mode: "detail"} + data := &Data{Mode: "detail", Truncated: clip.truncated, ShownOf: clip.issuesTotal} if row == nil { - // Unknown id: a detail pane with a nil Issue; the template shows a - // "not found" note and a link back to the board. + // Unknown id: a detail pane with a nil Issue. Whether that is an answer or + // an admission is Data.MissingBeyondCap's to tell — when the issues table + // was clipped at Max the id may simply live in the tail that was never + // read, and "no such issue" is a claim this projection cannot make. return data } @@ -287,8 +310,11 @@ func buildDetail( } // Comments are optional; a missing table just yields an empty thread. Each - // comment is also folded into the merged history timeline below. - if comments, _, err := readRowsOptional(ctx, sess, ref, "comments"); err == nil && comments != nil { + // comment is also folded into the merged history timeline below. A clipped + // comments table costs this issue's thread whatever sits past Max, so it + // counts towards the flag like any other input. + if comments, commentsTotal, err := readRowsOptional(ctx, sess, ref, "comments"); err == nil && comments != nil { + data.Truncated = data.Truncated || commentsTotal > Max ccols := indexCols(comments.Columns) for _, r := range comments.Rows { if cell(ccols, r, "issue_id") != want { @@ -310,7 +336,8 @@ func buildDetail( // The audit log (events) is optional too; when present it joins the comments // in the History tab as humanized, time-ordered entries. - if events, _, err := readRowsOptional(ctx, sess, ref, "events"); err == nil && events != nil { + if events, eventsTotal, err := readRowsOptional(ctx, sess, ref, "events"); err == nil && events != nil { + data.Truncated = data.Truncated || eventsTotal > Max ecols := indexCols(events.Columns) for _, r := range events.Rows { if cell(ecols, r, "issue_id") != want { diff --git a/beads/milestones.go b/beads/milestones.go index 335106aa0ed87f83f3b027f9a0a64774d458cf6c..9dd24b4455d8550f1a3e67f8733f3bcff6585e92 100644 --- a/beads/milestones.go +++ b/beads/milestones.go @@ -14,8 +14,23 @@ type MilestoneView struct { Milestones []MilestoneDetail Unlabeled int // issues carrying no milestone label Total int // all issues read + + // Truncated says one of the tables this rollup is computed from — issues, + // labels, dependencies, custom_statuses — exceeded Max and came back clipped, + // so every count below is arithmetic over a partial read. A clipped labels + // table is the quietest of them: membership itself goes missing, and a + // milestone can lose issues rather than merely undercount them. + Truncated bool + // ShownOf is the issues table's reported total, clipped or not. Total is what + // was read, ShownOf is what exists; they differ exactly when the issues read + // was clipped. + ShownOf int } +// IssuesClipped reports that the issues table itself exceeded Max, so this +// rollup covers only its first Max rows. +func (v *MilestoneView) IssuesClipped() bool { return v.ShownOf > Max } + // MilestoneDetail is one milestone's rollup and the issues under it, arranged // as a shallow hierarchy: the milestone-typed issue(s) first, then epics with // their subtasks nested one level below, then everything else. @@ -52,13 +67,19 @@ func (m MilestoneDetail) Pct() int { // label. An issue with several milestone labels counts under each. Missing // labels/statuses tables degrade to empty (no milestones), never an error. func BuildMilestones(ctx context.Context, sess BrowseSession, ref string) (*MilestoneView, error) { - issues, _, err := readRows(ctx, sess, ref, "issues") + issues, issuesTotal, err := readRows(ctx, sess, ref, "issues") if err != nil { return nil, err } - labels, _, _ := readRowsOptional(ctx, sess, ref, "labels") - statuses, _, _ := readRowsOptional(ctx, sess, ref, "custom_statuses") - deps, _, _ := readRowsOptional(ctx, sess, ref, "dependencies") + labels, labelsTotal, _ := readRowsOptional(ctx, sess, ref, "labels") + statuses, statusesTotal, _ := readRowsOptional(ctx, sess, ref, "custom_statuses") + deps, depsTotal, _ := readRowsOptional(ctx, sess, ref, "dependencies") + + // Every one of those four feeds the arithmetic below, so any one of them + // coming back clipped makes the rollup partial. The totals come back from the + // reads just made; nothing here reads a table twice to find out. + truncated := issuesTotal > Max || labelsTotal > Max || + statusesTotal > Max || depsTotal > Max // child issue → its parent-child parents; used to nest tasks under epics. parentsByChild := map[string][]string{} @@ -133,7 +154,13 @@ func BuildMilestones(ctx context.Context, sess BrowseSession, ref string) (*Mile out = append(out, *md) } - return &MilestoneView{Milestones: out, Unlabeled: unlabeled, Total: len(issues.Rows)}, nil + return &MilestoneView{ + Milestones: out, + Unlabeled: unlabeled, + Total: len(issues.Rows), + Truncated: truncated, + ShownOf: issuesTotal, + }, nil } // arrange splits a milestone's member cards into the display hierarchy: heads diff --git a/beads/model.go b/beads/model.go index 4e819b1d46351ba79d76daae06bfc3340fb2df42..3236ede8d0a393b2ef72c31ea8f5be41784ad3b8 100644 --- a/beads/model.go +++ b/beads/model.go @@ -28,11 +28,23 @@ type Data struct { Sections []Section // LayoutStream only: the same buckets, read top to bottom Counts Counts Total int // issues placed on the board (after filtering) - Truncated bool // an input table exceeded Max and was clipped - ShownOf int // when Truncated: the reported table total Filter Filter // active board filters (sticky form state) FilterOpts FilterOptions // distinct values for the filter dropdowns + // Truncated and ShownOf are set in every mode, board and detail alike: a + // projection that read only part of a table has to say so wherever it is + // rendered, or a partial answer reads as a complete one. + // + // Truncated says some table this projection read exceeded Max and came back + // clipped. Which tables that covers is the mode's own set — the board buckets + // from issues and dependencies; the detail pane also draws on labels, + // custom_statuses, comments and events. + Truncated bool + // ShownOf is the issues table's reported total, clipped or not — what exists, + // against the rows actually read. IssuesClipped is the comparison callers + // usually want. + ShownOf int + // Query is the request's query as parsed, carried so the layout toggle can // rebuild this exact URL with one key replaced (web's withQuery). The view // envelope does not carry the query, and rebuilding it from Filter would @@ -59,6 +71,24 @@ type Data struct { SubtaskTotal int // len(Subtasks); the progress denominator } +// IssuesClipped reports that the issues table itself exceeded Max, so this +// projection saw only its first Max rows. Truncated is the wider fact (any +// input table was clipped); this is the one that decides whether the issue set +// in hand is the whole tracker. +func (d *Data) IssuesClipped() bool { return d.ShownOf > Max } + +// Missing reports that a detail build did not find the requested issue: the +// pane has no Issue to render. It says nothing about why — MissingBeyondCap +// does. +func (d *Data) Missing() bool { return d.Mode == "detail" && d.Issue == nil } + +// MissingBeyondCap separates the two ways an issue can be missing. False with +// Missing set means the read was complete and there is no such issue. True +// means the issues table was clipped at Max and the id was not among the rows +// read — it may sit in the tail this projection never saw, and a surface that +// answers "no such issue" here is stating something it does not know. +func (d *Data) MissingBeyondCap() bool { return d.Missing() && d.IssuesClipped() } + // Filter holds the active board filters, parsed from the query string and // echoed back into the form so selections stick across submits. Empty fields // mean "no constraint". diff --git a/beads/truncation_test.go b/beads/truncation_test.go new file mode 100644 index 0000000000000000000000000000000000000000..f69ca75e25ebbab3d753137313266f5499b5463c --- /dev/null +++ b/beads/truncation_test.go @@ -0,0 +1,309 @@ +package beads + +import ( + "context" + "fmt" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-dolt/browse" +) + +// --- a session that clips the way the store does ----------------------------- + +// clippingSession is a BrowseSession that honours offset/limit and reports the +// table's true row count, which is what a real read at limit=Max does: the +// first Max rows, and a Total that says the rest exists. +// +// The clip is performed by the seam rather than written into a fixture by hand. +// A page whose Total simply disagrees with its own Rows would let a test assert +// the flag while never producing the situation the flag is about — and the tail +// row, which is the whole point of the "not in what was read" case, has to be +// genuinely absent from the rows handed to the projection. +type clippingSession struct { + rowsByTable map[string]*browse.RowPage +} + +func (s *clippingSession) Rows(_ context.Context, _, table string, offset, limit int) (*browse.RowPage, error) { + p, ok := s.rowsByTable[table] + if !ok { + return nil, fmt.Errorf("%w: %s", browse.ErrTableNotFound, table) + } + total := len(p.Rows) + lo := offset + if lo > total { + lo = total + } + hi := total + if limit > 0 && lo+limit < hi { + hi = lo + limit + } + return &browse.RowPage{Columns: p.Columns, Rows: p.Rows[lo:hi], Offset: lo, Total: total}, nil +} + +// --- fixtures past the cap --------------------------------------------------- + +// manyIssues builds n issue rows, ids i-0000… in order, cycling through the +// three status categories. With n > Max the ids from index Max on are the tail +// a capped read never sees. +func manyIssues(n int) *browse.RowPage { + statuses := []string{"open", "in_progress", "closed"} + rows := make([][]string, 0, n) + for i := 0; i < n; i++ { + rows = append(rows, []string{ + issueID(i), + fmt.Sprintf("Issue %d", i), + statuses[i%len(statuses)], + "1", + "task", + "alice", + fmt.Sprintf("2024-01-01 00:00:%02d", i%60), + "0", + }) + } + return &browse.RowPage{ + Columns: []string{"id", "title", "status", "priority", "issue_type", "assignee", "created_at", "is_blocked"}, + Rows: rows, + Total: n, + } +} + +func issueID(i int) string { return fmt.Sprintf("i-%04d", i) } + +func statusRows() *browse.RowPage { + return &browse.RowPage{ + Columns: []string{"name", "category"}, + Rows: [][]string{ + {"open", "open"}, + {"in_progress", "in_progress"}, + {"closed", "closed"}, + }, + Total: 3, + } +} + +// bigTracker is a tracker of Max+5 issues: five of them exist only past the cap. +// Every other table stays small, so a flag raised over this fixture can only +// have come from the issues read. +// +// Two issues carry milestone:m1 — i-0003, which a capped read sees, and +// i-2003, which it does not. The rollup over a clipped read therefore reports +// half a milestone, which is the arithmetic this fixture is here to catch. +func bigTracker() *clippingSession { + return &clippingSession{rowsByTable: map[string]*browse.RowPage{ + "issues": manyIssues(Max + 5), + "dependencies": { + Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"}, + Rows: [][]string{{"d1", "i-0007", "i-0001", "blocks"}}, + Total: 1, + }, + "labels": { + Columns: []string{"issue_id", "label"}, + Rows: [][]string{ + {"i-0003", "milestone:m1"}, + {"i-2003", "milestone:m1"}, + {"i-0007", "backend"}, + }, + Total: 3, + }, + "custom_statuses": statusRows(), + }} +} + +// smallTrackerWith is three issues and no clip anywhere, plus whatever extra +// tables a test wants to oversize. It isolates the clip to one optional table. +func smallTrackerWith(extra map[string]*browse.RowPage) *clippingSession { + tables := map[string]*browse.RowPage{ + "issues": manyIssues(3), + "dependencies": { + Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"}, + Rows: [][]string{{"d1", "i-0001", "i-0000", "blocks"}}, + Total: 1, + }, + "custom_statuses": statusRows(), + } + for name, page := range extra { + tables[name] = page + } + return &clippingSession{rowsByTable: tables} +} + +// manyComments builds n comment rows, all on i-0000. +func manyComments(n int) *browse.RowPage { + rows := make([][]string, 0, n) + for i := 0; i < n; i++ { + rows = append(rows, []string{"i-0000", "alice", fmt.Sprintf("comment %d", i), "2024-01-05 00:00:00"}) + } + return &browse.RowPage{ + Columns: []string{"issue_id", "author", "text", "created_at"}, + Rows: rows, + Total: n, + } +} + +func detail(t *testing.T, sess BrowseSession, id string) *Data { + t.Helper() + d, err := Build(context.Background(), sess, "main", url.Values{"issue": {id}}) + require.NoError(t, err) + require.Contains(t, []string{"detail", "epic"}, d.Mode) + return d +} + +// --- the detail branch ------------------------------------------------------- + +func TestDetailReportsAClippedIssuesRead(t *testing.T) { + d := detail(t, bigTracker(), "i-0007") + + require.NotNil(t, d.Issue, "i-0007 is inside the first Max rows") + assert.Equal(t, "i-0007", d.Issue.ID) + assert.True(t, d.Truncated, "the issues table exceeded Max") + assert.True(t, d.IssuesClipped()) + assert.Equal(t, Max+5, d.ShownOf, "the tracker's true issue count, not the number read") + assert.False(t, d.Missing()) + assert.False(t, d.MissingBeyondCap()) +} + +func TestDetailIssueInTheTailIsNotAbsence(t *testing.T) { + sess := bigTracker() + d := detail(t, sess, "i-2003") + + // The issue exists in the tracker... + rows := sess.rowsByTable["issues"].Rows + assert.Equal(t, "i-2003", rows[2003][0]) + // ...but not in the rows this projection read, and it says so. + assert.Nil(t, d.Issue) + assert.True(t, d.Missing()) + assert.True(t, d.MissingBeyondCap(), "the read was clipped, so absence is not established") + assert.True(t, d.IssuesClipped()) + assert.Equal(t, Max+5, d.ShownOf) +} + +func TestDetailAbsentIssueOnACompleteReadIsAbsence(t *testing.T) { + d := detail(t, beadsFixture(), "i-nope") + + assert.Nil(t, d.Issue) + assert.True(t, d.Missing()) + assert.False(t, d.MissingBeyondCap(), "nothing was clipped, so the id genuinely does not exist") + assert.False(t, d.Truncated) + assert.False(t, d.IssuesClipped()) + assert.Equal(t, 4, d.ShownOf) +} + +func TestDetailReportsAClippedCommentsRead(t *testing.T) { + d := detail(t, smallTrackerWith(map[string]*browse.RowPage{ + "comments": manyComments(Max + 3), + }), "i-0000") + + require.NotNil(t, d.Issue) + assert.Len(t, d.Comments, Max, "the thread is the comments that were read") + assert.True(t, d.Truncated, "the comments table was clipped, so the thread is partial") + assert.False(t, d.IssuesClipped(), "the issue set itself is complete") + assert.Equal(t, 3, d.ShownOf) +} + +func TestDetailReportsAClippedLabelsRead(t *testing.T) { + labels := make([][]string, 0, Max+2) + for i := 0; i < Max+2; i++ { + labels = append(labels, []string{"i-0002", fmt.Sprintf("label-%04d", i)}) + } + sess := smallTrackerWith(map[string]*browse.RowPage{ + "labels": {Columns: []string{"issue_id", "label"}, Rows: labels, Total: len(labels)}, + }) + + d := detail(t, sess, "i-0000") + require.NotNil(t, d.Issue) + assert.True(t, d.Truncated, "labels feed the detail pane, so a clipped labels read is partial too") + assert.False(t, d.IssuesClipped()) +} + +// --- the board's flag is what it always was ---------------------------------- + +func TestBoardTruncationIsUnchanged(t *testing.T) { + t.Run("clipped issues", func(t *testing.T) { + d, err := Build(context.Background(), bigTracker(), "main", url.Values{}) + require.NoError(t, err) + require.Equal(t, "board", d.Mode) + assert.True(t, d.Truncated) + assert.Equal(t, Max+5, d.ShownOf) + assert.Equal(t, Max, d.Counts.Total, "the board holds the rows that were read") + assert.False(t, d.Missing(), "a board is never a miss") + }) + + t.Run("clipped dependencies", func(t *testing.T) { + deps := make([][]string, 0, Max+1) + for i := 0; i < Max+1; i++ { + deps = append(deps, []string{fmt.Sprintf("d%d", i), "i-0001", "i-0000", "blocks"}) + } + sess := smallTrackerWith(map[string]*browse.RowPage{ + "dependencies": { + Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"}, + Rows: deps, + Total: len(deps), + }, + }) + d, err := Build(context.Background(), sess, "main", url.Values{}) + require.NoError(t, err) + assert.True(t, d.Truncated, "dependencies is one of the two tables the board buckets from") + assert.False(t, d.IssuesClipped(), "and Truncated is not the same fact as a clipped issue set") + assert.Equal(t, 3, d.ShownOf) + }) + + t.Run("a clip in a table the board does not bucket from", func(t *testing.T) { + // The board's flag has always named issues and dependencies. Widening it to + // labels or comments would change what the board reports; the detail pane + // covers them because it reads them for this one issue. + d, err := Build(context.Background(), smallTrackerWith(map[string]*browse.RowPage{ + "comments": manyComments(Max + 3), + }), "main", url.Values{}) + require.NoError(t, err) + assert.False(t, d.Truncated) + assert.False(t, d.IssuesClipped()) + }) +} + +// --- the milestone rollup ---------------------------------------------------- + +func TestMilestonesReportAClippedRead(t *testing.T) { + v, err := BuildMilestones(context.Background(), bigTracker(), "main") + require.NoError(t, err) + + assert.True(t, v.Truncated, "the rollup below is arithmetic over a partial read") + assert.True(t, v.IssuesClipped()) + assert.Equal(t, Max, v.Total, "issues read") + assert.Equal(t, Max+5, v.ShownOf, "issues that exist") + + require.Len(t, v.Milestones, 1) + assert.Equal(t, 1, v.Milestones[0].Total, + "m1's other member is i-2003, which sits past the cap — the count is short and the view says why") +} + +func TestMilestonesOnACompleteReadReportNoClip(t *testing.T) { + v, err := BuildMilestones(context.Background(), milestoneFixture(), "main") + require.NoError(t, err) + + assert.False(t, v.Truncated) + assert.False(t, v.IssuesClipped()) + assert.Equal(t, 6, v.Total) + assert.Equal(t, v.Total, v.ShownOf, "nothing was left behind, so the two agree") +} + +func TestMilestonesReportAClippedLabelsRead(t *testing.T) { + labels := make([][]string, 0, Max+2) + labels = append(labels, []string{"i-0000", "milestone:m1"}) + for i := 0; i < Max+1; i++ { + labels = append(labels, []string{"i-0002", fmt.Sprintf("label-%04d", i)}) + } + sess := smallTrackerWith(map[string]*browse.RowPage{ + "labels": {Columns: []string{"issue_id", "label"}, Rows: labels, Total: len(labels)}, + }) + + v, err := BuildMilestones(context.Background(), sess, "main") + require.NoError(t, err) + assert.True(t, v.Truncated, "membership itself was read partially") + assert.False(t, v.IssuesClipped(), "though every issue was read") + assert.Equal(t, 3, v.Total) + assert.Equal(t, 3, v.ShownOf) +}